revert: "revert: perf: napi communication (#11132) (#11162)" #11166
revert: "revert: perf: napi communication (#11132) (#11162)" #11166
Conversation
✅ Deploy Preview for rspack canceled.
|
|
📝 Benchmark detail: Open
|
📦 Binary Size-limit
🎉 Size decreased by 7.00KB from 49.04MB to 49.03MB (⬇️0.01%) |
CodSpeed Performance ReportMerging #11166 will not alter performanceComparing Summary
|
|
📝 Benchmark detail: Open
Threshold exceeded: ["10000_production-mode_persistent-hot + exec","large-dyn-imports_production-mode + exec"] |
|
📝 Benchmark detail: Open
Threshold exceeded: ["10000_production-mode_persistent-hot + exec","large-dyn-imports_production-mode + exec"] |
|
📝 Benchmark detail: Open
Threshold exceeded: ["large-dyn-imports_production-mode + exec"] |
|
📝 Benchmark detail: Open
|
There was a problem hiding this comment.
Pull Request Overview
This PR reverts a previous revert to restore NAPI communication optimizations (#11132) due to performance degradation observed with UkeyMap compared to FxHashMap. The root cause appears to be UkeyMap's non-optimized handling of non-contiguous data storage, leading to memory fragmentation and reduced cache locality.
- Switches from structured object passing to string-based JSON serialization for NAPI communication
- Implements thread-local property buffers to reduce memory allocations
- Refactors module connection methods to use arrays instead of vectors for better performance
Reviewed Changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rspack/src/taps/normalModuleFactory.ts | Changes after-resolve callback to accept JSON string instead of structured object |
| packages/rspack/src/builtin-plugin/ExternalsPlugin.ts | Adds resolve request caching and refactors to use instance methods |
| packages/rspack/etc/core.api.md | Updates ExternalsPlugin.raw() method signature to remove compiler parameter |
| crates/rspack_binding_api/src/resource_data.rs | Implements thread-local property buffer for resource data serialization |
| crates/rspack_binding_api/src/raw_options/raw_external.rs | Optimizes external item context handling with Arc sharing and async cleanup |
| crates/rspack_binding_api/src/plugins/interceptor.rs | Changes after-resolve hook to use JSON string serialization |
| crates/rspack_binding_api/src/normal_module_factory.rs | Adds Serialize derives to data structures |
| crates/rspack_binding_api/src/modules/*.rs | Implements thread-local property buffers for module serialization |
| crates/rspack_binding_api/src/module_graph.rs | Optimizes connection methods to use arrays and reusable buffers |
| crates/rspack_binding_api/src/dependency.rs | Changes dependency IDs getter to return arrays instead of vectors |
| crates/rspack_binding_api/src/compilation/mod.rs | Optimizes modules getter to use arrays |
| crates/rspack_binding_api/src/build_info.rs | Implements thread-local property buffer for build info |
| crates/rspack_binding_api/Cargo.toml | Adds rayon dependency for async cleanup |
Summary
Performance degradation observed when switching from FxHashMap to UkeyMap.
The efficiency loss stems from the tag-based SIMD optimization failure in hashbrown's lookup mechanism.
References:
hashbrown/src/raw/mod.rs at master · rust-lang/hashbrown
hashbrown/src/control/tag.rs at master · rust-lang/hashbrown
hashbrown/src/control/group/generic.rs at master · rust-lang/hashbrown
Related links
Checklist