Repository navigation
perf(hasher): reduce redundant file hashing work - #1257
Open
djordjem88 wants to merge 3 commits into
Open
djordjem88 wants to merge 3 commits into
djordjem88 wants to merge 3 commits into
Conversation
Skip empty native calls and reuse pre-hash metadata. Invalidate stale entries and surface native hashing failures explicitly, with regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b26a45b-25aa-45e3-95b4-f9a1b55dcb58
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b26a45b-25aa-45e3-95b4-f9a1b55dcb58
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b26a45b-25aa-45e3-95b4-f9a1b55dcb58
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Hash failures are logged without settling worker requests, leaving cacheable targets waiting indefinitely and hanging the build.
3 open issues
What changed in this PR
Reduces redundant work in Lage’s file-hashing layer and improves cache consistency when files change during hashing. This is a self-contained optimization, separate from the broader experimental cache-hit work.
Changes:
- Skips native calls for empty inputs and native hashing for fully cached inputs.
- Reuses pre-hash metadata, invalidates stale entries, and makes null native results explicit failures.
- Adds focused regression tests and a patch release note.
| File | Description |
|---|---|
packages/hasher/src/FileHasher.ts |
Adds fast paths, metadata reuse, cache invalidation, and explicit native-result errors. |
packages/hasher/src/__tests__/FileHasher.test.ts |
Covers cache reuse, metadata changes, hashing races, normalized paths, and failure recovery. |
change/change-3d55df9e-d087-4b46-9a27-4aa36592561c.json |
Records the hasher improvements and changed failure behavior as a patch release. |
Review effort: Max
Comment on lines
+123
to
+124
| if (!updatedHashes) { | ||
| throw new Error("Failed to hash files"); |
Comment on lines
+129
to
+130
| // Evict the native result's normalized key; input paths may use a different form. | ||
| delete this.#store[file]; |
| if (hash == null) { | ||
| // Evict the native result's normalized key; input paths may use a different form. | ||
| delete this.#store[file]; | ||
| throw new Error(`Failed to hash file "${file}"`); |
This branch has not been deployed
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.
Summary
Requested reviewers: Nemanja Tesic (@nemanjatesic) and Elizabeth Craig (@ecraig12345).
Skip native stat/hash calls for empty inputs and skip native hashing when every requested file is already cached. Reuse pre-hash metadata instead of calling
statSyncagain after hashing, so an edit during hashing is not paired with newer metadata and incorrectly reused on the next lookup.Stale entries are evicted before refresh. A valid hash without matching pre-hash metadata is returned without caching it.
Part of the broader Lage performance effort explored in #1238. This change is self-contained and based on current
main; it does not depend on the other performance PRs.Behavior change for review
This is not exclusively a performance change. A null native batch result now throws
Failed to hash filesinstead of being treated as an empty result. A nullish individual native hash now throws instead of being converted to an empty string. These explicit failures were deliberately retained for reviewer discussion; previous incidental filesystem failures could already throw, but these native-result cases broaden the error behavior.Validation
mainatcf155216without conflicts; the hasher implementation and tests are unchanged by the sync.