+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

feat: replace the system allocator with mimalloc on Windows #2900

Merged
merged 1 commit into from
Jul 19, 2022
Merged

feat: replace the system allocator with mimalloc on Windows #2900

merged 1 commit into from
Jul 19, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Jul 19, 2022

Summary

This change adds the mimalloc crate as a dependency of rome_cli, rome_lsp and xtask_bench, and uses the provided MiMalloc allocator in the resulting binaries instead of the default (system) allocator on Windows

Since the benchmark actions are being run on Linux, here is the improvements I measured when running all the benchmark suites on my machine:

group                                    mimalloc                               sysalloc
-----                                    --------                               --------
analyzer/css.js                          1.00      2.2±0.13ms     5.3 MB/sec    2.41      5.3±1.75ms     2.2 MB/sec
analyzer/index.js                        1.00      4.5±0.34ms     7.2 MB/sec    2.42     10.9±2.81ms     3.0 MB/sec
analyzer/lint.ts                         1.00      3.2±0.18ms    12.9 MB/sec    1.64      5.3±0.59ms     7.8 MB/sec
analyzer/parser.ts                       1.00      7.1±0.40ms     6.8 MB/sec    1.89     13.3±2.31ms     3.6 MB/sec
analyzer/router.ts                       1.00      4.8±0.30ms    12.8 MB/sec    2.13     10.2±3.03ms     6.0 MB/sec
analyzer/statement.ts                    1.00      6.1±0.35ms     5.8 MB/sec    1.91     11.6±2.20ms     3.0 MB/sec
analyzer/typescript.ts                   1.00      9.0±0.46ms     6.1 MB/sec    2.04     18.3±5.51ms     3.0 MB/sec

formatter/checker.ts                     1.00   320.2±25.53ms     8.1 MB/sec    1.46   466.7±31.38ms     5.6 MB/sec
formatter/compiler.js                    1.00   204.9±18.59ms     5.1 MB/sec    1.57   321.9±28.05ms     3.3 MB/sec
formatter/d3.min.js                      1.00   167.0±12.69ms  1607.1 KB/sec    1.57   262.3±27.08ms  1023.1 KB/sec
formatter/dojo.js                        1.00     12.3±1.36ms     5.6 MB/sec    1.52     18.7±3.06ms     3.7 MB/sec
formatter/ios.d.ts                       1.00   232.8±41.46ms     8.0 MB/sec    1.41   328.7±25.71ms     5.7 MB/sec
formatter/jquery.min.js                  1.00    64.8±20.42ms  1305.7 KB/sec    2.01   130.5±19.46ms   648.6 KB/sec
formatter/math.js                        1.00   346.7±37.09ms  1912.6 KB/sec    1.50   520.3±38.18ms  1274.4 KB/sec
formatter/parser.ts                      1.00      7.8±0.73ms     6.2 MB/sec    1.45     11.3±0.78ms     4.3 MB/sec
formatter/pixi.min.js                    1.00   192.5±21.07ms     2.3 MB/sec    1.53   293.9±28.54ms  1529.3 KB/sec
formatter/react-dom.production.min.js    1.00    83.2±21.17ms  1415.7 KB/sec    1.07    89.0±65.80ms  1323.5 KB/sec
formatter/react.production.min.js        1.00      4.6±0.63ms  1355.2 KB/sec    1.78      8.3±1.68ms   763.4 KB/sec
formatter/router.ts                      1.00      7.1±1.86ms     8.6 MB/sec    1.22      8.6±0.57ms     7.1 MB/sec
formatter/tex-chtml-full.js              1.00   443.7±47.14ms     2.1 MB/sec    1.56   694.0±51.20ms  1344.5 KB/sec
formatter/three.min.js                   1.00   214.0±15.02ms     2.7 MB/sec    1.59   339.6±32.35ms  1770.5 KB/sec
formatter/typescript.js                  1.00  1297.5±80.09ms     7.3 MB/sec    1.27  1651.0±188.23ms     5.8 MB/sec
formatter/vue.global.prod.js             1.00   125.7±10.00ms   981.7 KB/sec    1.58   198.7±35.41ms   620.8 KB/sec

parser/checker.ts                        1.00   200.1±21.68ms    13.0 MB/sec    1.37   273.5±29.17ms     9.5 MB/sec
parser/compiler.js                       1.00   119.4±14.05ms     8.8 MB/sec    1.09   130.0±45.31ms     8.1 MB/sec
parser/d3.min.js                         1.00     78.0±7.40ms     3.4 MB/sec    1.07    83.4±14.63ms     3.1 MB/sec
parser/dojo.js                           1.00      7.1±1.03ms     9.7 MB/sec    1.10      7.8±2.79ms     8.8 MB/sec
parser/ios.d.ts                          1.00   174.9±16.95ms    10.7 MB/sec    1.33   231.8±32.61ms     8.0 MB/sec
parser/jquery.min.js                     1.00     21.5±7.30ms     3.9 MB/sec    1.31     28.0±6.12ms     2.9 MB/sec
parser/math.js                           1.00   136.2±18.57ms     4.8 MB/sec    1.13   154.2±51.88ms     4.2 MB/sec
parser/parser.ts                         1.00      4.5±0.53ms    10.7 MB/sec    1.53      6.9±2.46ms     7.0 MB/sec
parser/pixi.min.js                       1.08     95.9±9.92ms     4.6 MB/sec    1.00    88.5±37.16ms     5.0 MB/sec
parser/react-dom.production.min.js       1.17     32.9±4.86ms     3.5 MB/sec    1.00     28.1±8.53ms     4.1 MB/sec
parser/react.production.min.js           1.35      2.1±0.77ms     2.9 MB/sec    1.00  1579.7±369.79µs     3.9 MB/sec
parser/router.ts                         1.00      3.7±0.61ms    16.7 MB/sec    1.45      5.3±0.86ms    11.5 MB/sec
parser/tex-chtml-full.js                 1.00   195.6±19.67ms     4.7 MB/sec    1.02   198.8±60.05ms     4.6 MB/sec
parser/three.min.js                      1.00    97.8±12.48ms     6.0 MB/sec    1.13   110.1±40.18ms     5.3 MB/sec
parser/typescript.js                     1.00   790.8±73.79ms    12.0 MB/sec    1.45  1144.7±97.97ms     8.3 MB/sec
parser/vue.global.prod.js                1.12     43.1±4.82ms     2.8 MB/sec    1.00    38.6±10.96ms     3.1 MB/sec

Test Plan

Not sure if this is applicable ? to give more context, the allocator is such a fundamental part of the language it would probably be easy to detect it if it were broken, but on the other hand we don't run any test on Windows

@leops leops requested a review from ematipico as a code owner July 19, 2022 12:24
@leops leops requested a review from a team July 19, 2022 12:24
@leops leops temporarily deployed to aws July 19, 2022 12:24 Inactive
@ematipico
Copy link
Contributor

ematipico commented Jul 19, 2022 •
edited
Loading

but on the other hand we don't run any test on Windows

Should we really do it? It would makes sense from my point of view. We should add macOS too.

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 88c306d
Status: ✅  Deploy successful!
Preview URL: https://f18b2064.tools-8rn.pages.dev
Branch Preview URL: https://feature-mimalloc.tools-8rn.pages.dev

View logs

@github-actions
Copy link

Playground for commit fdcc21f

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45878 45878 0
Passed 44938 44938 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 388 388 0
Failed 5558 5558 0
Panics 0 0 0
Coverage 6.53% 6.53% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12393 12393 0
Failed 3864 3864 0
Panics 0 0 0
Coverage 76.23% 76.23% 0.00%

@leops
Copy link
Contributor Author

leops commented Jul 19, 2022

but on the other hand we don't run any test on Windows

Should we really do it? It would makes sense from my point of view. We should add macOS too.

Outside of these kind of low-level changes the abstractions provided by the Rust standard library should shield us from the underlying platform. For the parts of the codebase that don't really interact with the OS like the parser, formatter or analyzer, a given test case is either going to succeed on all of Linux, macOS and Windows, or fail on all three platforms

@leops leops merged commit 5ab824d into main Jul 19, 2022
@leops leops deleted the feature/mimalloc branch July 19, 2022 12:54
@MichaReiser MichaReiser mentioned this pull request Feb 11, 2023
Merged
@UnknownPlatypus UnknownPlatypus mentioned this pull request Mar 16, 2025
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ematipico ematipico ematipico approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@leops @ematipico
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载