Overlap the two reads when comparing binary snapshots - #1889
Merged
Conversation
The verified and received streams were filled strictly one after the other, in 8KB chunks, so a passing run read both files end to end sequentially for every binary snapshot. The two streams are independent, so the fills now overlap, which roughly halves the wall time. The chunk is 64KB, still under the large object heap threshold and inside the array pool's buckets, so there are 8x fewer round trips. On net6.0 and above the Memory overload of ReadAsync avoids the Task allocation the byte[] overload makes per call against a FileStream opened for async IO. The equal sized chunk alignment the length comparison depends on is unchanged, and WhenAll rather than sequential awaits so a failure on one side cannot leave the other unobserved. docs/comparer.md is regenerated from the snippet.
StreamComparerBenchmarks measures the previous commit against the implementation it replaced, kept in LegacyStreamComparer with the buffer size and read overload lifted to parameters. The four rungs isolate each part of the change: 8KB sequential byte[] reads, then 64KB, then the Memory overload, then the shipped overlapped version. Both sides are real async FileStreams opened the way IoHelpers.OpenRead opens them, and each size carries its own baseline so the ratios compare like for like. The 1MB compare drops to 0.13 of the old time and a ninth of the allocation, and 64KB to 0.26. At 2KB the buffer size does nothing, one read either way, so the whole 0.77 there is the overlapping. A mismatch that exits on the first chunk is 0.51. The buffered category is a MemoryStream against a FileStream, not a MemoryStream against another one. InnerCompare always opens the verified side with IoHelpers.OpenRead, so a pair of MemoryStreams never reaches the comparer, and measuring that shape reported a regression that cannot occur. Against the real shape it is 0.36. MixedEqualSpanningMultipleBuffers covers that buffered shape, which had only not-equal coverage. EqualWithShortReads covers the accumulation loop in ReadBufferAsync: a local FileStream returns the full buffer every time, so replacing that loop with a single read leaves every other test passing. A stream that always reads short keeps both sides chunk aligned under test, where a desync would surface as a spurious NotEqual on a network share.
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.
The verified and received streams were filled strictly one after the other, in 8KB chunks, so a passing run read both files end to end sequentially for every binary snapshot.
The two streams are independent, so the fills now overlap, which roughly halves the wall time. The chunk is 64KB, still under the large object heap threshold and inside the array pool's buckets, so there are 8x fewer round trips. On net6.0 and above the Memory overload of ReadAsync avoids the Task allocation the byte[] overload makes per call against a FileStream opened for async IO.
The equal sized chunk alignment the length comparison depends on is unchanged, and WhenAll rather than sequential awaits so a failure on one side cannot leave the other unobserved. docs/comparer.md is regenerated from the snippet.