Skip to content

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

Merged
merged 12 commits into from
Jun 18, 2025

Conversation

LingyuCoder
Copy link
Contributor

@LingyuCoder LingyuCoder commented Jun 12, 2025

Summary

  • Introduce PrefetchedExportsInfoUsed to prevent fetching unnecessary named exports which caused the performance degression. Then modify the param of get_used_name to enum to support PrefetchedExportsInfoUsed for fast path when the names list is empty.
  • Unify the way of accessing properties. Set all properties of ExportsInfoData/ExportInfoData to private and provide getter/setter to access them.
  • Move get_target/find_target related functions to exports/target.rs.
  • Move referenced export related functions to exports/referenced_export.rs, and also rename process_export_info to collect_referenced_export_items to make the function name more informative

Generate by AI

This 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:

  • Replaced direct field access in ExportInfoData with getter methods like info.name() and info.global_used() for better encapsulation and maintainability in crates/rspack_core/src/exports/export_info_getter.rs. [1] [2] [3] [4] [5]
  • Updated get_used_name and get_used methods to use the new getter methods, ensuring consistent access patterns. [1] [2]

Code Simplification and Refactoring:

  • Introduced GetUsedNameParam::WithNames for better parameter clarity in ExportsInfoGetter::get_used_name calls. [1] [2] [3]
  • Replaced FindTargetRetEnum with FindTargetResult for more descriptive enum variants in impl ConcatenatedModule.

Prefetching and Hash Optimization:

  • Replaced prefetch calls with prefetch_used_info_without_name or get_prefetched_exports_info to improve export information retrieval and reduce unnecessary data fetching. [1] [2] [3] [4] [5]
  • Optimized hash computation by batching module graph hash calculations into a single loop, reducing redundant operations. [1] [2]

Enum and API Updates:

  • Updated ExportInfoGetter methods to utilize the new getter-based API for accessing export details, aligning with the encapsulation changes. [1] [2]
  • Improved readability and maintainability by replacing old enums with more descriptive ones, such as FindTargetResult.

Dependency and Import Adjustments:

  • Adjusted imports across files to include new dependencies like PrefetchExportsInfoMode and GetUsedNameParam. [1] [2] [3]

These changes collectively improve the codebase's clarity, performance, and maintainability.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copy link

netlify bot commented Jun 12, 2025

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit 217ccf2
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/6852412ad4889a0008c3afea
😎 Deploy Preview https://deploy-preview-10652--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@LingyuCoder LingyuCoder changed the title refactor: prefetch exports info data of getters part 2 refactor: prefetch exports info data of getters part 3 Jun 12, 2025
Copy link
Contributor Author

LingyuCoder commented Jun 12, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 21d6d37 to 585c448 Compare June 12, 2025 11:25
Copy link

codspeed-hq bot commented Jun 12, 2025

CodSpeed Performance Report

Merging #10652 will not alter performance

Comparing refactor/exports-info-getter-3 (217ccf2) with main (7dc2b87)

🎉 Hooray! codspeed-rust just leveled up to 2.7.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 10 untouched benchmarks
🆕 6 new benchmarks
⁉️ 2 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

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

@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 50a7d02 to 68568f5 Compare June 13, 2025 08:32
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 13, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 13, 2025
@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 68568f5 to 5ac9e42 Compare June 13, 2025 09:12
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 13, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 13, 2025
@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 0eb0228 to 328121c Compare June 16, 2025 03:39
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 16, 2025
@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 328121c to 0a5bacf Compare June 16, 2025 06:25
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 16, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 16, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 16, 2025
@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 350a1e5 to 84aac7d Compare June 16, 2025 12:24
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 16, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 17, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 17, 2025
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 17, 2025
@LingyuCoder LingyuCoder force-pushed the refactor/exports-info-getter-3 branch from 84aac7d to 9bfc925 Compare June 17, 2025 06:03
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 17, 2025
Copy link
Contributor

github-actions bot commented Jun 17, 2025

📝 Benchmark detail: Open

Name Base (2025-06-17 d520fea) Current Change
10000_big_production-mode_disable-minimize + exec 35.1 s ± 424 ms 36.5 s ± 757 ms +3.80 %
10000_development-mode + exec 1.88 s ± 13 ms 1.86 s ± 42 ms -1.23 %
10000_development-mode_hmr + exec 710 ms ± 24 ms 704 ms ± 22 ms -0.92 %
10000_production-mode + exec 2.31 s ± 37 ms 2.35 s ± 68 ms +1.82 %
10000_production-mode_persistent-cold + exec 2.49 s ± 45 ms 2.49 s ± 36 ms -0.06 %
10000_production-mode_persistent-hot + exec 1.75 s ± 40 ms 1.76 s ± 19 ms +0.67 %
arco-pro_development-mode + exec 1.79 s ± 111 ms 1.84 s ± 46 ms +2.67 %
arco-pro_development-mode_hmr + exec 370 ms ± 1.1 ms 370 ms ± 1.8 ms +0.18 %
arco-pro_production-mode + exec 3.37 s ± 99 ms 3.41 s ± 76 ms +1.05 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.52 s ± 79 ms 3.55 s ± 221 ms +0.79 %
arco-pro_production-mode_persistent-cold + exec 3.49 s ± 115 ms 3.47 s ± 76 ms -0.53 %
arco-pro_production-mode_persistent-hot + exec 2.12 s ± 40 ms 2.12 s ± 44 ms -0.13 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.39 s ± 58 ms 3.46 s ± 160 ms +1.81 %
large-dyn-imports_development-mode + exec 2.07 s ± 34 ms 2.09 s ± 33 ms +1.04 %
large-dyn-imports_production-mode + exec 2.06 s ± 38 ms 2.1 s ± 49 ms +2.29 %
threejs_development-mode_10x + exec 1.46 s ± 30 ms 1.47 s ± 24 ms +0.71 %
threejs_development-mode_10x_hmr + exec 830 ms ± 9.2 ms 838 ms ± 8.6 ms +1.01 %
threejs_production-mode_10x + exec 4.93 s ± 56 ms 4.97 s ± 52 ms +0.94 %
threejs_production-mode_10x_persistent-cold + exec 5.07 s ± 161 ms 5.07 s ± 15 ms +0.01 %
threejs_production-mode_10x_persistent-hot + exec 4.38 s ± 46 ms 4.42 s ± 36 ms +0.99 %
10000_big_production-mode_disable-minimize + rss memory 9236 MiB ± 54.5 MiB 9423 MiB ± 237 MiB +2.02 %
10000_development-mode + rss memory 687 MiB ± 29.3 MiB 677 MiB ± 33.3 MiB -1.40 %
10000_development-mode_hmr + rss memory 831 MiB ± 26 MiB 824 MiB ± 35.2 MiB -0.88 %
10000_production-mode + rss memory 694 MiB ± 54 MiB 710 MiB ± 86.9 MiB +2.27 %
10000_production-mode_persistent-cold + rss memory 818 MiB ± 58 MiB 798 MiB ± 53.1 MiB -2.46 %
10000_production-mode_persistent-hot + rss memory 766 MiB ± 28 MiB 751 MiB ± 66.2 MiB -2.01 %
arco-pro_development-mode + rss memory 615 MiB ± 76.6 MiB 616 MiB ± 49.7 MiB +0.12 %
arco-pro_development-mode_hmr + rss memory 540 MiB ± 53.5 MiB 550 MiB ± 43.4 MiB +1.90 %
arco-pro_production-mode + rss memory 743 MiB ± 109 MiB 735 MiB ± 113 MiB -0.96 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 740 MiB ± 65.4 MiB 781 MiB ± 59.3 MiB +5.53 %
arco-pro_production-mode_persistent-cold + rss memory 834 MiB ± 92.6 MiB 843 MiB ± 120 MiB +1.06 %
arco-pro_production-mode_persistent-hot + rss memory 675 MiB ± 52.5 MiB 671 MiB ± 60.1 MiB -0.53 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 713 MiB ± 90.8 MiB 731 MiB ± 58.2 MiB +2.51 %
large-dyn-imports_development-mode + rss memory 703 MiB ± 6.28 MiB 705 MiB ± 16.8 MiB +0.35 %
large-dyn-imports_production-mode + rss memory 574 MiB ± 8.16 MiB 577 MiB ± 13.6 MiB +0.43 %
threejs_development-mode_10x + rss memory 710 MiB ± 25.2 MiB 698 MiB ± 12.7 MiB -1.72 %
threejs_development-mode_10x_hmr + rss memory 873 MiB ± 76.9 MiB 861 MiB ± 16.4 MiB -1.33 %
threejs_production-mode_10x + rss memory 959 MiB ± 50.6 MiB 942 MiB ± 33.6 MiB -1.72 %
threejs_production-mode_10x_persistent-cold + rss memory 1056 MiB ± 42.7 MiB 1040 MiB ± 48.9 MiB -1.46 %
threejs_production-mode_10x_persistent-hot + rss memory 817 MiB ± 49.7 MiB 833 MiB ± 33.5 MiB +1.89 %

@web-infra-dev web-infra-dev deleted a comment from github-actions bot Jun 17, 2025
@LingyuCoder LingyuCoder marked this pull request as ready for review June 18, 2025 03:01
@LingyuCoder LingyuCoder requested a review from JSerFeng as a code owner June 18, 2025 03:01
@LingyuCoder LingyuCoder requested a review from ahabhgk June 18, 2025 03:01
@LingyuCoder LingyuCoder enabled auto-merge (squash) June 18, 2025 04:28
.get_used_name(&module_graph, *runtime, ids)
.map(|used| used.is_inlined())
.unwrap_or_default()
&& {
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Contributor

@ahabhgk ahabhgk left a 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

@LingyuCoder LingyuCoder merged commit dbb9bb4 into main Jun 18, 2025
42 of 45 checks passed
@LingyuCoder LingyuCoder deleted the refactor/exports-info-getter-3 branch June 18, 2025 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants