Skip to content
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

fix: patched mimalloc to use MADV_DONTNEED than MADV_FREE on Linux #9037

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

h-a-n-a
Copy link
Contributor

@h-a-n-a h-a-n-a commented Jan 17, 2025

Summary

Use patched mimalloc to use MADV_DONTNEED instead of MADV_FREE on reseting memory.
This also takes effect on macOS.

While MADV_FREE is faster than MADV_DONTNEED, it does not affect the RSS as MADV_DONTNEED does.
To reiterate, using MADV_DONTNEED means process-level memory statistics like RSS will be more
accurately reflect the amount of physical memory being used by rspack.

Even this does not show any evidence that it would cause OOM issues, the fact seems stay the opposite
with reported incidents in go. GoLang team and many have encountered this issue and have reverted the option to MADV_DONTNEED in golang/go#42330.

Rspack uses mimalloc for memory management.
The default strategy of mimalloc@2 is to purge the memory instead of reseting them.
However, in some cases, it would still reset memory (mi_free_block_mt, freeing blocks multi-thread):
image

With this patch, MADV_FREE is replaced with MADV_DONTNEED:
image

Related: microsoft/mimalloc#776
Rspack related: #8976

Checklist

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

Copy link

netlify bot commented Jan 17, 2025

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit 2c39c4f
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/678a25720f1c6a00083822da
😎 Deploy Preview https://deploy-preview-9037--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 site configuration.

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Jan 17, 2025
Copy link
Contributor

github-actions bot commented Jan 17, 2025

📝 Benchmark detail: Open

Name Base (2025-01-17 2cb67d1) Current Change
10000_big_production-mode_disable-minimize + exec 37.5 s ± 811 ms 38.8 s ± 507 ms +3.49 %
10000_development-mode + exec 1.86 s ± 165 ms 1.84 s ± 49 ms -1.06 %
10000_development-mode_hmr + exec 687 ms ± 25 ms 683 ms ± 18 ms -0.65 %
10000_production-mode + exec 2.43 s ± 122 ms 2.41 s ± 135 ms -0.60 %
10000_production-mode_persistent-cold + exec 2.52 s ± 19 ms 2.52 s ± 50 ms -0.11 %
10000_production-mode_persistent-hot + exec 1.77 s ± 23 ms 1.76 s ± 26 ms -0.76 %
arco-pro_development-mode + exec 1.78 s ± 111 ms 1.74 s ± 90 ms -2.55 %
arco-pro_development-mode_hmr + exec 388 ms ± 6.1 ms 387 ms ± 2.5 ms -0.37 %
arco-pro_production-mode + exec 3.78 s ± 221 ms 3.68 s ± 191 ms -2.77 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.72 s ± 187 ms 3.68 s ± 142 ms -1.08 %
arco-pro_production-mode_persistent-cold + exec 3.92 s ± 119 ms 3.88 s ± 213 ms -1.03 %
arco-pro_production-mode_persistent-hot + exec 2.47 s ± 107 ms 2.49 s ± 181 ms +0.79 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.76 s ± 91 ms 3.69 s ± 177 ms -1.99 %
large-dyn-imports_development-mode + exec 2.1 s ± 26 ms 2.08 s ± 59 ms -1.08 %
large-dyn-imports_production-mode + exec 2.14 s ± 29 ms 2.15 s ± 17 ms +0.44 %
threejs_development-mode_10x + exec 1.62 s ± 36 ms 1.61 s ± 21 ms -0.32 %
threejs_development-mode_10x_hmr + exec 760 ms ± 13 ms 771 ms ± 34 ms +1.41 %
threejs_production-mode_10x + exec 5.48 s ± 193 ms 5.54 s ± 334 ms +1.21 %
threejs_production-mode_10x_persistent-cold + exec 5.52 s ± 50 ms 5.62 s ± 467 ms +1.86 %
threejs_production-mode_10x_persistent-hot + exec 4.68 s ± 344 ms 4.71 s ± 87 ms +0.60 %
10000_big_production-mode_disable-minimize + rss memory 9567 MiB ± 503 MiB 8700 MiB ± 42.7 MiB -9.06 %
10000_development-mode + rss memory 662 MiB ± 6.56 MiB 659 MiB ± 11.1 MiB -0.46 %
10000_development-mode_hmr + rss memory 1399 MiB ± 344 MiB 1368 MiB ± 51.1 MiB -2.25 %
10000_production-mode + rss memory 674 MiB ± 55.3 MiB 646 MiB ± 14.1 MiB -4.16 %
10000_production-mode_persistent-cold + rss memory 763 MiB ± 46.4 MiB 744 MiB ± 12.2 MiB -2.49 %
10000_production-mode_persistent-hot + rss memory 776 MiB ± 54.8 MiB 741 MiB ± 28.5 MiB -4.54 %
arco-pro_development-mode + rss memory 570 MiB ± 60.3 MiB 570 MiB ± 34.1 MiB +0.04 %
arco-pro_development-mode_hmr + rss memory 590 MiB ± 69.9 MiB 676 MiB ± 81.9 MiB +14.52 %
arco-pro_production-mode + rss memory 748 MiB ± 52.4 MiB 722 MiB ± 23.1 MiB -3.55 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 719 MiB ± 92.8 MiB 724 MiB ± 35.6 MiB +0.66 %
arco-pro_production-mode_persistent-cold + rss memory 819 MiB ± 45.7 MiB 847 MiB ± 17.3 MiB +3.41 %
arco-pro_production-mode_persistent-hot + rss memory 682 MiB ± 31.6 MiB 707 MiB ± 21.9 MiB +3.68 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 710 MiB ± 87 MiB 718 MiB ± 23.9 MiB +1.16 %
large-dyn-imports_development-mode + rss memory 636 MiB ± 5.9 MiB 646 MiB ± 5.61 MiB +1.56 %
large-dyn-imports_production-mode + rss memory 531 MiB ± 6.77 MiB 528 MiB ± 6.94 MiB -0.52 %
threejs_development-mode_10x + rss memory 567 MiB ± 24 MiB 528 MiB ± 7.73 MiB -6.99 %
threejs_development-mode_10x_hmr + rss memory 1139 MiB ± 227 MiB 1144 MiB ± 97.1 MiB +0.37 %
threejs_production-mode_10x + rss memory 870 MiB ± 67.3 MiB 816 MiB ± 20.4 MiB -6.18 %
threejs_production-mode_10x_persistent-cold + rss memory 980 MiB ± 54.2 MiB 944 MiB ± 34.2 MiB -3.66 %
threejs_production-mode_10x_persistent-hot + rss memory 904 MiB ± 46.5 MiB 844 MiB ± 30.2 MiB -6.57 %

Copy link

codspeed-hq bot commented Jan 17, 2025

CodSpeed Performance Report

Merging #9037 will not alter performance

Comparing mimalloc-downgrade (2c39c4f) with main (8d6406d)

Summary

✅ 3 untouched benchmarks

@h-a-n-a h-a-n-a marked this pull request as ready for review January 17, 2025 10:21
Copy link
Member

@chenjiahan chenjiahan left a comment

Choose a reason for hiding this comment

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

👍

@h-a-n-a h-a-n-a merged commit c47c243 into main Jan 17, 2025
57 checks passed
@h-a-n-a h-a-n-a deleted the mimalloc-downgrade branch January 17, 2025 11:00
chenjiahan added a commit that referenced this pull request Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants