-
-
Notifications
You must be signed in to change notification settings - Fork 764
refactor: prefetch exports info data of getters part 3 #10652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for rspack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
21d6d37
to
585c448
Compare
CodSpeed Performance ReportMerging #10652 will not alter performanceComparing 🎉 Hooray!
|
Benchmark | BASE |
HEAD |
Change | |
---|---|---|---|---|
rust@bundle-basic |
4 ms | N/A | N/A | |
rust@bundle-basic_sourcemap |
4.2 ms | N/A | N/A | |
🆕 | bundle@basic-react-development |
N/A | 8.2 ms | N/A |
🆕 | bundle@basic-react-production |
N/A | 11.1 ms | N/A |
🆕 | bundle@basic-react-production-sourcemap |
N/A | 11.9 ms | N/A |
🆕 | bundle@threejs-development |
N/A | 956.4 ms | N/A |
🆕 | bundle@threejs-production |
N/A | 1.9 s | N/A |
🆕 | bundle@threejs-production-sourcemap |
N/A | 2.4 s | N/A |
50a7d02
to
68568f5
Compare
68568f5
to
5ac9e42
Compare
0eb0228
to
328121c
Compare
328121c
to
0a5bacf
Compare
350a1e5
to
84aac7d
Compare
84aac7d
to
9bfc925
Compare
📝 Benchmark detail: Open
|
3d8d2d1
to
217ccf2
Compare
.get_used_name(&module_graph, *runtime, ids) | ||
.map(|used| used.is_inlined()) | ||
.unwrap_or_default() | ||
&& { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see there are many similar pattern like this, a few lines of code refactored to many lines of code, can we have a facade function for this pattern, to make more cleaner at the call site, and more easier to use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good to me, a suggestion can resolve in the future PR
Summary
PrefetchedExportsInfoUsed
to prevent fetching unnecessary named exports which caused the performance degression. Then modify the param ofget_used_name
to enum to supportPrefetchedExportsInfoUsed
for fast path when the names list is empty.ExportsInfoData/ExportInfoData
to private and provide getter/setter to access them.get_target/find_target
related functions toexports/target.rs
.exports/referenced_export.rs
, and also renameprocess_export_info
tocollect_referenced_export_items
to make the function name more informativeThis pull request introduces multiple changes across several files to enhance the handling of export information and improve code efficiency. The updates include replacing direct field access with getter methods, introducing new enums for clarity, and optimizing hash computation. Below is a breakdown of the most important changes grouped by theme.
Enhancements to Export Information Handling:
ExportInfoData
with getter methods likeinfo.name()
andinfo.global_used()
for better encapsulation and maintainability incrates/rspack_core/src/exports/export_info_getter.rs
. [1] [2] [3] [4] [5]get_used_name
andget_used
methods to use the new getter methods, ensuring consistent access patterns. [1] [2]Code Simplification and Refactoring:
GetUsedNameParam::WithNames
for better parameter clarity inExportsInfoGetter::get_used_name
calls. [1] [2] [3]FindTargetRetEnum
withFindTargetResult
for more descriptive enum variants inimpl ConcatenatedModule
.Prefetching and Hash Optimization:
prefetch
calls withprefetch_used_info_without_name
orget_prefetched_exports_info
to improve export information retrieval and reduce unnecessary data fetching. [1] [2] [3] [4] [5]Enum and API Updates:
ExportInfoGetter
methods to utilize the new getter-based API for accessing export details, aligning with the encapsulation changes. [1] [2]FindTargetResult
.Dependency and Import Adjustments:
PrefetchExportsInfoMode
andGetUsedNameParam
. [1] [2] [3]These changes collectively improve the codebase's clarity, performance, and maintainability.
Checklist