Repository navigation
[patch] Compare natural strings in place, allocating nothing per comparison - #64
Merged
Merged
Conversation
…arison Compare split both strings into a List<Chunk> through a StringBuilder before comparing, so every comparison allocated two lists, a builder per string and a string per chunk: about 616 bytes. A sort makes O(n log n) comparisons. It now walks both strings a chunk at a time by index. Digits are read by their value with CharUnicodeInfo, text chunks compare with string.CompareOrdinal, and a number against text compares its first digit's ASCII equivalent with the text's first character. The ordering is unchanged: a differential run of 2,000,000 random pairs (ASCII, Arabic-Indic, Devanagari and mathematical digits, leading zeros, lone surrogates, emoji) against the previous implementation found no difference in sign. Sorting 50,000 file names went from 532 ms to 115 ms in Release. Fixes #56 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zULxdhphqscknnLMm1Bk1
|
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
2 participants
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.
Fixes #56
Summary
#57 already removed the per-call
Regex, butComparestill built both strings' chunks before comparing them: aList<Chunk>and aStringBuilderper string, plus a string per chunk. That came to about 616 bytes per comparison. This PR implements the issue's preferred fix, a character scanner that walks both strings chunk by chunk and allocates nothing.CharUnicodeInfoand compared by value. Non-ASCII digits, non-BMP digits, leading zeros and long digit runs behave exactly as before.string.CompareOrdinal(x, xStart, y, yStart, length)plus a length tie-break. That matches the previousstring.Compare(..., Ordinal)on the extracted chunks.Chunkrecord andusing System.Textare gone. The public API is unchanged.Verification
Compare_AllocatesNothing: it runs 14,000 comparisons over ASCII, non-ASCII, surrogate-pair, leading-zero and number-against-text inputs and assertsGC.GetAllocatedBytesForCurrentThread()didn't move. With the library change stashed it fails (8624000 bytes allocated over 14000 comparisons). With the change it passes.Comparefrom the previous implementation and this one on 2,000,000 random pairs. The pairs mixed ASCII, Arabic-Indic, Devanagari and mathematical digits, letters, punctuation, emoji and lone surrogates. There were 0 mismatches.List.Sortof 50,000IMG_<n>_<n>.jpgnames): 532 ms before, 115 ms after.🤖 Generated with Claude Code
https://claude.ai/code/session_015zULxdhphqscknnLMm1Bk1
Generated by Claude Code