Build checker cache keys in an inline buffer with one-shot hashing - #4784
Open
mds-ant wants to merge 1 commit into
Open
Build checker cache keys in an inline buffer with one-shot hashing#4784mds-ant wants to merge 1 commit into
mds-ant wants to merge 1 commit into
Conversation
Cache keys were built through a by-value `xxh3.Hasher` embedded in `keyBuilder`. That is a streaming hasher with a large internal block buffer, so every key build zeroed a sizeable struct on the stack and pushed each ID through the streaming `Write` path, even though a typical key is only a handful of bytes. `keyBuilder` now accumulates the key bytes in a small inline buffer, spilling to a slice for the rare oversized key, and hashes them with the one-shot `xxh3.Hash128`. The byte stream is unchanged (`hashWrite32`/`hashWrite64` become `writeUint32`/`writeUint64` with the same little-endian layout), so every key value is identical to before.
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes checker cache-key construction by buffering small keys inline and hashing them in one shot.
Changes:
- Replaces streaming hashing with a 192-byte inline buffer and overflow path.
- Preserves little-endian key encoding.
- Adds boundary and overflow encoding tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/checker/checker.go |
Implements buffered cache-key construction and one-shot hashing. |
internal/checker/keybuilder_test.go |
Verifies key encoding and buffer-boundary behavior. |
Member
|
I swear Claude is addicted to introducing one off test files @typescript-bot perf test this |
Contributor
Author
I actually asked it to add a test to make sure that the resulting hash is still identical. Happy to get rid of it if you don't think it's worth keeping. |
Member
|
I'm on the fence 😄 I'm actually triggering all these perf runs on my phone, so haven't seen the test file without terrible word wrapping; my immediate uncertainty was just the maintenance cost. But I'll check A lot of this key builder stuff might just go away in a future version of Go when they add maps that have custom hashers. |
Member
|
Mainly, we already have |
jakebailey
reviewed
Jul 29, 2026
jakebailey
reviewed
Jul 29, 2026
Comment on lines
+17363
to
+17365
| inlineLength int | ||
| overflowBuffer []byte | ||
| inlineBuffer [192]byte |
Member
There was a problem hiding this comment.
Wouldn't this be a lot easier to just initialize it with say:
type keyBuilder struct {
buffer []byte
space [192]byte
}
func (b *keyBuilder) init() {
if b.buffer == nil {
b.buffer = b.space[:0]
}
}
func (b *keyBuilder) writeByte(c byte) {
b.init()
// ...
}And let Go manage overflowing?
Member
There was a problem hiding this comment.
(or a new func, or even a pool get/put)
Contributor
Author
There was a problem hiding this comment.
Unfortunately, the self-referential slice defeat's Go's escape analysis, which means every var b keyBuilder gets moved to the heap. On the VS Code corpus that's +10.7M allocations and a significant wall time regression.
Member
There was a problem hiding this comment.
Hm. Theoretically it could just go onto the heap anyway in a pool
mds-ant
force-pushed
the
perf/checker-oneshot-key-hash
branch
from
July 29, 2026 15:39
b9d8773 to
e143e5a
Compare
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
lspComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Context
Every structural cache key in the checker is built through
keyBuilder, whose only field was a by-valuexxh3.Hasher. That's a streaming hasher with an internal block buffer of over a kilobyte, so everyvar b keyBuilderzeroed the whole struct on the stack and fed each ID through the streamingWritepath, even though a typical key is about a dozen bytes andSum128on such input just one-shot-hashes the buffered bytes anyway.This PR
keyBuildernow accumulates the key's bytes in a smallinlineBuffer, spilling to a heapoverflowBufferonly for the rare oversized key, and hashes them with the one-shotxxh3.Hash128. ThehashWrite32/hashWrite64helpers becomewriteUint32/writeUint64methods with the same little-endian layout, so the byte stream is unchanged and every key value is identical to before.On a full check of VS Code the keys average 11.5 bytes with a p99.9 of 129 bytes. Only 0.04% exceed the 192-byte buffer, so the spill path is effectively cold.
Performance
This PR was assisted by Claude Code.