refactor: use `std::sync::Mutex` instead of `parking_lot::Mutex` #3095

IWANABETHATGUY · 2022-08-23T07:23:35Z

Summary

Part of ☂️ Tracking issues or potential improvement we could make after upgrades to the Rust toolchain #3054
With the standard library Mutex and RwLock being const-constructible it's now possible to use these in the DiffReport of the Prettier test suite

Test Plan

Passing CI

ematipico

Another change that should be part of this PR is removing lazy_static around the definition of METRICS.

Now that Mutex can be defined as const, the usage of lazy_static is not needed anymore.

IWANABETHATGUY · 2022-08-23T08:42:28Z

Seems not, rustc doesn't allow us to create a const Mutex that inner value can't create in compile time.
rustc emit errors below:

cannot call non-const fn `HashMap::<CallsiteKey, Mutex<CallsiteEntry>>::new` in constants
calls in constants are limited to constant functions, tuple structs and tuple variants

xunilrj · 2022-08-23T08:46:10Z

!bench_analyzer

xunilrj · 2022-08-23T08:46:18Z

!bench_formatter

github-actions · 2022-08-23T08:56:53Z

Analyzer Benchmark Results

group                     main                                   pr
-----                     ----                                   --
analyzer/css.js           1.00      2.2±0.00ms     5.2 MB/sec    1.00      2.2±0.00ms     5.3 MB/sec
analyzer/index.js         1.00      6.0±0.03ms     5.4 MB/sec    1.01      6.0±0.07ms     5.4 MB/sec
analyzer/lint.ts          1.00      3.2±0.01ms    13.1 MB/sec    1.00      3.2±0.01ms    13.1 MB/sec
analyzer/parser.ts        1.01      7.4±0.15ms     6.6 MB/sec    1.00      7.4±0.03ms     6.6 MB/sec
analyzer/router.ts        1.01      5.2±0.08ms    11.8 MB/sec    1.00      5.2±0.04ms    11.9 MB/sec
analyzer/statement.ts     1.01      7.0±0.04ms     5.1 MB/sec    1.00      7.0±0.02ms     5.1 MB/sec
analyzer/typescript.ts    1.00     10.5±0.10ms     5.2 MB/sec    1.01     10.6±0.10ms     5.1 MB/sec

ematipico · 2022-08-23T09:07:10Z

Seems not, rustc doesn't allow us to create a const Mutex that inner value can't create in compile time. rustc emit errors below:
cannot call non-const fn `HashMap::<CallsiteKey, Mutex<CallsiteEntry>>::new` in constants
calls in constants are limited to constant functions, tuple structs and tuple variants

Really difficult to give feedback without seeing the code, but removing only lazy_static is not correct, we have to do some refactor.

IWANABETHATGUY · 2022-08-23T09:12:33Z

pile time. rustc emit errors below:

IWANABETHATGUY · 2022-08-23T09:14:44Z

The only way I know to create a hashmap in compile time is phf

MichaReiser · 2022-08-23T09:14:45Z

Seems not, rustc doesn't allow us to create a const Mutex that inner value can't create in compile time. rustc emit errors below:
cannot call non-const fn `HashMap::<CallsiteKey, Mutex<CallsiteEntry>>::new` in constants
calls in constants are limited to constant functions, tuple structs and tuple variants
Really difficult to give feedback without seeing the code, but removing only lazy_static is not correct, we have to do some refactor.

The problem is that RwLock stores a HashMap and HashMap::new is not const. So you can write RwLock::new(...) but there's no way to create the map.

github-actions · 2022-08-23T09:24:03Z

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.06   512.2±12.60ms     5.1 MB/sec    1.00   484.9±10.47ms     5.4 MB/sec
formatter/compiler.js                    1.01    317.5±8.22ms     3.3 MB/sec    1.00    313.8±7.57ms     3.3 MB/sec
formatter/d3.min.js                      1.01    258.8±9.30ms  1037.2 KB/sec    1.00    255.8±7.42ms  1049.4 KB/sec
formatter/dojo.js                        1.02     16.6±0.74ms     4.1 MB/sec    1.00     16.3±0.59ms     4.2 MB/sec
formatter/ios.d.ts                       1.03   320.9±11.40ms     5.8 MB/sec    1.00    310.7±7.25ms     6.0 MB/sec
formatter/jquery.min.js                  1.00     69.8±2.31ms  1212.2 KB/sec    1.00     69.6±3.64ms  1216.0 KB/sec
formatter/math.js                        1.02   542.7±12.78ms  1221.9 KB/sec    1.00   531.0±17.10ms  1248.6 KB/sec
formatter/parser.ts                      1.02     10.8±0.34ms     4.5 MB/sec    1.00     10.6±0.31ms     4.6 MB/sec
formatter/pixi.min.js                    1.02   305.2±17.96ms  1472.3 KB/sec    1.00    298.1±9.10ms  1507.4 KB/sec
formatter/react-dom.production.min.js    1.00     84.9±3.38ms  1387.8 KB/sec    1.00     84.9±5.15ms  1388.1 KB/sec
formatter/react.production.min.js        1.01      4.1±0.16ms  1553.0 KB/sec    1.00      4.0±0.13ms  1570.6 KB/sec
formatter/router.ts                      1.01      8.3±0.21ms     7.4 MB/sec    1.00      8.2±0.33ms     7.5 MB/sec
formatter/tex-chtml-full.js              1.02   691.1±12.60ms  1350.3 KB/sec    1.00   680.7±23.69ms  1370.9 KB/sec
formatter/three.min.js                   1.01   334.3±10.94ms  1798.2 KB/sec    1.00    331.7±9.14ms  1812.6 KB/sec
formatter/typescript.js                  1.02       2.1±0.02s     4.6 MB/sec    1.00       2.0±0.03s     4.7 MB/sec
formatter/vue.global.prod.js             1.01    113.9±4.38ms  1083.0 KB/sec    1.00    113.1±6.04ms  1090.7 KB/sec

refactor: 💡 use std::sync::Mutex instead of parking_lot

851f5da

IWANABETHATGUY requested review from leops, ematipico and MichaReiser as code owners August 23, 2022 07:23

IWANABETHATGUY requested a review from a team August 23, 2022 07:23

chore: 🤖 update lock

69feed2

ematipico reviewed Aug 23, 2022

View reviewed changes

MichaReiser approved these changes Aug 23, 2022

View reviewed changes

MichaReiser merged commit e9eb7a0 into rome:main Aug 23, 2022

IWANABETHATGUY deleted the refactor/mutex branch August 23, 2022 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: use `std::sync::Mutex` instead of `parking_lot::Mutex` #3095

refactor: use `std::sync::Mutex` instead of `parking_lot::Mutex` #3095

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

ematipico left a comment •

edited

Loading

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

xunilrj commented Aug 23, 2022

Uh oh!

xunilrj commented Aug 23, 2022

Uh oh!

github-actions bot commented Aug 23, 2022

Uh oh!

ematipico commented Aug 23, 2022

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

MichaReiser commented Aug 23, 2022

Uh oh!

github-actions bot commented Aug 23, 2022

Uh oh!

Uh oh!

refactor: use std::sync::Mutex instead of parking_lot::Mutex #3095

refactor: use std::sync::Mutex instead of parking_lot::Mutex #3095

Uh oh!

Conversation

IWANABETHATGUY commented Aug 23, 2022

Summary

Test Plan

Uh oh!

ematipico left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

xunilrj commented Aug 23, 2022

Uh oh!

xunilrj commented Aug 23, 2022

Uh oh!

github-actions bot commented Aug 23, 2022

Analyzer Benchmark Results

Uh oh!

ematipico commented Aug 23, 2022

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

IWANABETHATGUY commented Aug 23, 2022

Uh oh!

MichaReiser commented Aug 23, 2022

Uh oh!

github-actions bot commented Aug 23, 2022

Formatter Benchmark Results

Uh oh!

Uh oh!

refactor: use `std::sync::Mutex` instead of `parking_lot::Mutex` #3095

refactor: use `std::sync::Mutex` instead of `parking_lot::Mutex` #3095

ematipico left a comment •

edited

Loading