Repository navigation
Correct performance and option claims in the README - #394
Merged
Merged
Conversation
Several statements no longer matched the code or measurement.
The "up to 10x faster" claim had no qualifier. The speedup comes from
chunking parts concurrently, so it scales with core count: on a 4-core
machine make measured 1.8x-3.6x faster than casync 2, and single
threaded desync is in casync's range or slightly behind, since its
rolling hash is pure Go against casync's optimized C. Say so, rather
than leaving readers to expect 10x anywhere.
The per-command table called all-zero data the worst case for make and
"likely comparable to casync". The mechanism it describes is real, but
nothing supports the comparison, so keep the former and drop the latter.
Also:
- info defaults to json output, not plain
- the chunker accepts a min chunk size equal to the rolling hash
window, not only larger than it (chunker.go:135)
- the timeout store option is only consulted by the HTTP(S), S3 and
OCI stores; GCS, SFTP and local ignore it
- device numbers on macOS were fixed in #388, so only the filemode
half of the catar interop caveat still stands
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documentation only. Several statements no longer matched the code or measurement.
Performance claims
"up to 10x faster" had no qualifier. The speedup comes from chunking parts concurrently, so it scales with core count. On a 4-core machine
makemeasured 1.8x–3.6x faster than casync 2 (Debian2+20201210-2+b2), both writing zstd-compressed chunks to a local store. Run single-threaded, desync is in casync's range and can be slightly behind, since its rolling hash is pure Go against casync's optimized C — the advantage is concurrency, not a faster inner loop. The new paragraph says all of that, so a reader on a small machine isn't left expecting 10x.The 0-byte row claimed
makeis "Slow (worst-case) — likely comparable to casync". The mechanism behind it is real — concurrent chunkers can't align when there are no split points — but the comparison to casync isn't supported by anything. Measured, desync was faster than casync on all-zero data, though that number doesn't settle it either: an all-zero file dedups to a single chunk, so the compress-and-store half ofmakeis nearly free for both tools and the alignment penalty is masked. The row now keeps the mechanism and drops the unsupported comparison. Measuring a mostly-zero file with unique data mixed in would isolate it properly; that hasn't been done.The "identical output to casync" bullet now says byte-identical, which #393 made literally true for blob indexes.
Four smaller corrections, each checked against the code
infodefaults toplainjson(desync info --help). Fixed the doc, not the flag — changing the default would break anyone parsing today's output.chunker.go:135rejectsmin < ChunkerWindowSize, so exactly 48 is acceptedtimeoutstore option, no scope givenremotehttp.go,oci.goand (since #390)s3.goconsult it. GCS, SFTP and local ignore it, so setting it there does nothing.Not included
S3 and GCS silently ignore
--ca-cert,--trust-insecureand the client certificate options — onlyremotehttp.goandoci.gobuild a TLS config. That's a bug with a contained fix rather than something to document, so it's left for its own PR.