improve unordered byte span comparison performance#89
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new autogram solver variant (AutogramBytesNoStringsV5g) that uses a new unordered byte-span comparison helper (UnorderedByteSpanEqualsWithSum), and wires the new solver/comparison into the existing benchmark and test suites. It also adjusts ByteHistoryKey64.GetHashCode() as a performance optimization.
Changes:
- Added
AutogramBytesNoStringsV5gand a newReadOnlySpan<byte>extension methodUnorderedByteSpanEqualsWithSum. - Integrated the new solver and comparison method into benchmarks and added a correctness test for V5g.
- Simplified
ByteHistoryKey64.GetHashCode()implementation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| AutogramTest/AutogramTest.cs | Adds a new unit test covering the V5g solver. |
| AutogramBenchmark/UnorderedByteArrayComparisonBenchmark.cs | Adds a benchmark for the new unordered-span comparison method. |
| AutogramBenchmark/AutogramBenchmark.cs | Adds setup/cleanup and a benchmark entry for the V5g solver. |
| Autogram/Extensions.cs | Introduces UnorderedByteSpanEqualsWithSum for faster unordered comparisons. |
| Autogram/ByteHistoryKey64.cs | Updates hash code computation for ByteHistoryKey64. |
| Autogram/AutogramBytesNoStringsV5g.cs | New solver implementation using the new comparison method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+218
to
+223
| Span<int> counts = stackalloc int[256]; | ||
|
|
||
| foreach (var b1 in a) | ||
| { | ||
| counts[b1]++; | ||
| } |
| HashCode.Combine(length, chunk0, chunk1, chunk2), | ||
| HashCode.Combine(chunk3, chunk4, chunk5, chunk6), | ||
| chunk7); | ||
| return HashCode.Combine(chunk0, chunk1, chunk2, chunk3, chunk4, chunk5, chunk6, chunk7); |
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.
This pull request introduces a new implementation,
AutogramBytesNoStringsV5g, which enhances the autogram search algorithm by utilizing a new method for comparing byte arrays (UnorderedByteSpanEqualsWithSum). The update also integrates this new version into the benchmarking and testing infrastructure, and optimizes a hash code computation. The most important changes are grouped below:New Algorithm Implementation
AutogramBytesNoStringsV5g, which uses theUnorderedByteSpanEqualsWithSummethod for improved byte array comparison during the autogram search process. This class is now part of the solver suite.Byte Array Comparison Enhancements
UnorderedByteSpanEqualsWithSumextension method forReadOnlySpan<byte>, which first compares the sum of elements before checking for unordered equality, improving efficiency in certain scenarios.UnorderedByteSpanEqualsWithSuminUnorderedByteArrayComparisonBenchmark.csto measure its performance.Benchmarking and Testing Integration
AutogramBytesNoStringsV5ginto the benchmarking suite: added setup, cleanup, and a benchmark method to evaluate its performance alongside existing solvers. [1] [2] [3] [4]AutogramBytesNoStringsV5gin the test suite to ensure correctness.Code Optimization
GetHashCodemethod inByteHistoryKey64by simplifying the hash computation for better performance and maintainability.