Improve StringBuilder.Equals - #133167
Conversation
Compare the overlapping portions of each chunk from the end using ReadOnlySpan<char>.SequenceEqual instead of walking the chunk arrays one character at a time. The approach is based on the logic used in StringBuilder.Equals(ReadOnlySpan<char>).
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke |
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The new implementation introduces small-input performance regressions that should be mitigated (or at least clearly justified) in an optimization-focused change.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors StringBuilder.Equals(StringBuilder?) to compare chunk contents in larger blocks (using ReadOnlySpan<char>.SequenceEqual) rather than comparing one character at a time, aiming to improve throughput for larger builders and multi-chunk layouts.
Changes:
- Adds a fast-path for the single-chunk case using
SequenceEqual, and an early last-character mismatch check. - Moves the multi-chunk comparison logic into a new helper (
EqualsCore) that compares overlapping portions of each chunk from the end viaSequenceEqual.
File summaries
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs | Reworks StringBuilder.Equals implementation to compare chunk tails using span-based SequenceEqual and introduces a core helper for multi-chunk comparison. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
| if (m_ChunkPrevious == null && sb.m_ChunkPrevious == null) | ||
| { | ||
| return m_ChunkChars.AsSpan(0, thisChunkLength).SequenceEqual(sb.m_ChunkChars.AsSpan(0, sbChunkLength)); | ||
| } |
| int length = Math.Min(thisChunkLength, sbChunkLength); | ||
| if (!thisChunk.m_ChunkChars.AsSpan(thisChunkLength - length, length) | ||
| .SequenceEqual(sbChunk.m_ChunkChars.AsSpan(sbChunkLength - length, length))) | ||
| { |
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
Compare the overlapping portions of each chunk from the end using
ReadOnlySpan<char>.SequenceEqualinstead of walking the chunk arrays one character at a time.The approach is based on the logic used in
StringBuilder.Equals(ReadOnlySpan<char>).Benchmark
The results of the following benchmarks on local environment, run on the
mainandprbranches, are summarized below.In the cases where
Size = 1andSize = 4, some overhead occurs. In many cases, performance improves when theSizeis 16 or higher.Sorry for posting so many benchmarks.
Benchmark result
Benchmark source