refactor: persistent cache remove lock#13589
Merged
jerrykingxyz merged 1 commit intomainfrom Apr 3, 2026
Merged
Conversation
Merging this PR will improve performance by 2.87%🎉 Hooray!
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | rust@create_chunk_ids |
10.8 ms | 10.5 ms | +2.87% |
Comparing jerry/cache (e8fb391) with main (713b798)
Footnotes
-
19 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the persistent cache storage API to remove internal locking by switching storage mutation APIs to require &mut access, and by passing storage references explicitly through snapshot/occasion flows rather than sharing Arc handles.
Changes:
- Refactor
rspack_storage::Storageto use&mut selffor mutating operations (set/remove/save/reset) and introduceBoxStoragein place ofArc<dyn Storage>. - Update persistent cache components (snapshot, build deps, occasions, cache hooks) to accept
&dyn Storage/&mut dyn Storageparameters rather than storing clonableArcstorage handles. - Update rspack tools cache comparison utilities to work with
BoxStorageand borrowed storage references.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rspack_tools/src/compare/mod.rs | Switches comparison pipeline to BoxStorage and borrows storages for per-scope comparisons. |
| crates/rspack_tools/src/compare/snapshot.rs | Updates snapshot comparison to accept borrowed storages (&dyn Storage). |
| crates/rspack_tools/src/compare/occasion/meta.rs | Updates meta comparison to accept borrowed storages (&dyn Storage). |
| crates/rspack_tools/src/compare/occasion/make.rs | Refactors make comparison to use new MakeOccasion API (storage passed into recovery). |
| crates/rspack_storage/src/lib.rs | Changes Storage trait mutating methods to &mut self; replaces ArcStorage with BoxStorage. |
| crates/rspack_storage/src/memory/mod.rs | Removes Mutex and updates implementation to match new Storage trait mutability requirements. |
| crates/rspack_storage/src/filesystem/mod.rs | Removes Mutex around staged updates and updates APIs to &mut self mutation. |
| crates/rspack_core/src/cache/persistent/storage/mod.rs | Re-exports BoxStorage and changes create_storage to return boxed storage. |
| crates/rspack_core/src/cache/persistent/snapshot/mod.rs | Removes stored storage handle; requires explicit storage refs for add/remove/calc operations. |
| crates/rspack_core/src/cache/persistent/occasion/meta/mod.rs | Removes stored storage handle; passes storage into save/recovery. |
| crates/rspack_core/src/cache/persistent/occasion/make/mod.rs | Removes stored storage handle; passes storage into save/recovery. |
| crates/rspack_core/src/cache/persistent/occasion/make/module_graph.rs | Updates module graph save/recovery to use &mut dyn Storage / &dyn Storage. |
| crates/rspack_core/src/cache/persistent/build_dependencies/mod.rs | Threads storage references through add/validate to match snapshot API. |
| crates/rspack_core/src/cache/persistent/mod.rs | Updates PersistentCache to own BoxStorage and pass borrowed storage through all operations. |
| crates/rspack_core/src/cache/mod.rs | Changes cache hook after_build_module_graph to require &mut self. |
| crates/rspack_core/src/cache/mixed.rs | Adapts mixed cache implementation to the updated hook mutability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Rsdoctor Bundle Diff AnalysisFound 5 projects in monorepo, 0 projects with changes. 📊 Quick Summary
Generated by Rsdoctor GitHub Action |
Contributor
📦 Binary Size-limit
🎉 Size decreased by 9.59KB from 49.09MB to 49.08MB (⬇️0.02%) |
Contributor
|
📝 Benchmark detail: Open
|
LingyuCoder
approved these changes
Apr 2, 2026
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
trait Storage { - fn set(&self, scope: &'static str, key: Vec<u8>, value: Vec<u8>); + fn set(&mut self, scope: &'static str, key: Vec<u8>, value: Vec<u8>); - fn remove(&self, scope: &'static str, key: &[u8]); + fn remove(&mut self, scope: &'static str, key: &[u8]); ... }struct MakeOccassion { - storage: ArcStorage ... } impl MakeOccassion { - fn save(...) {} + fn save(storage: &mut dyn Storage, ...) {} ... }Related links
Checklist