Repository navigation
Reduce allocations in Luhn, Redact, and CLI sanitizer paths - #13
Merged
Merged
Conversation
Replaces the per-byte state machine with a loop that jumps straight past non-digit runs, avoiding redundant checks on text-heavy input. Assisted-By: cagent
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.
Profiling the hot paths in card/SSN validation, redaction, and the CLI's output sanitizer surfaced avoidable allocations and wasted CPU:
validLuhnbuilt a fresh[]intper call,Redactgrew itsstrings.Builderto the full input length regardless of whether the output would shrink or expand,sanitizeValueallocated a newstrings.Replaceron every multi-line match, and the digit-run prefilter walked every byte of long non-digit prose while maintainingrun/afterDigitstate it never used.This change lands four independent, behavior-preserving optimizations:
validLuhnscans digits in place from the end of the string instead of allocating a slice, and bails out once more than 19 digits are seen.Redactprecomputes the exact output size (falling back to the input length onintoverflow) sostrings.Builder.Growallocates once instead of resizing as it writes.sanitizeValuereuses a single package-levelstrings.Replacerinstead of constructing one per call; sharing it is safe sinceReplacer.Replaceholds no mutable state.hasSeparatedDigitRunskips runs of non-digit bytes with a tight inner loop instead of tracking separator state on every byte.Each commit is checked against a reference implementation via fuzzing (over 2.3M differential cases) plus unit tests, and the branch passes
go test -race -count=3 ./..., aGOTOOLCHAIN=go1.26.5 go test -race ./...run,go vet ./...,golangci-lint run ./...andgolangci-lint fmt --diff, andgo mod tidy -diff. A temporary baseline-vs-current harness additionally compared 18,157 inputs acrossContains/Redact/Find/ContainsBytes/FindBytesunder the race detector, and 19 CLI scenarios at 1/2/8/32 workers for exact stdout/stderr/exit-status parity, with no mismatches.On one Apple M4 Max machine running Go 1.27, interleaved benchmarks show the clean-input
Redact/Containspaths roughly 25% faster,validLuhndropping from 192 B and 2 allocations to zero, 64 KiB secret redaction dropping from about 74 KB to 248 B allocated, and the multiline sanitizer dropping from 10 to 3 allocations and running about 19% faster. These numbers are specific to that run and hardware, and are not a guarantee against regressions on other platforms. An earlier draft also buffered CLI output in abytes.Bufferfor fewer syscalls, but that was dropped after review because it could retain more queued-output memory, so it is not part of this PR. The existing Unicode gate mismatch and the unbounded CLI reorder buffer are known, unrelated issues left out of scope here.