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

perf(rome_formatter): Improve Printer source map performance #3104

Merged
merged 1 commit into from
Aug 24, 2022
Merged

perf(rome_formatter): Improve Printer source map performance #3104

merged 1 commit into from
Aug 24, 2022

Conversation

MichaReiser
Copy link
Contributor

Summary

This PR improves the formatters performance by only adding unique source markers inside the printer and adding take_* methods to take the source markers to avoid the need to copy all markers to a new vec.

Test Plan

cargo test. I added a new unit test for mapped markers in #3092 that verifies that only unique markers are added (and are mapped to the right location)

@MichaReiser
perf(rome_formatter): Improve Printer source map performance
86767e1 
This PR improves the formatters performance by only adding unique source markers inside the printer and adding `take_*` methods to take the source markers to avoid the need to copy all markers to a new vec.
@MichaReiser MichaReiser requested a review from ematipico as a code owner August 24, 2022 10:20
@MichaReiser MichaReiser temporarily deployed to aws August 24, 2022 10:20 Inactive
@MichaReiser
Copy link
Contributor Author

!bench_formatter

@github-actions
Copy link

Playground for commit df910ed

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 86767e1
Status: ✅  Deploy successful!
Preview URL: https://b7962fbb.tools-8rn.pages.dev
Branch Preview URL: https://perf-source-map-markers.tools-8rn.pages.dev

View logs

@ematipico
Copy link
Contributor

Would it be possible to have the unit test here?

@MichaReiser
Copy link
Contributor Author

MichaReiser commented Aug 24, 2022 •
edited
Loading

Would it be possible to have the unit test here?

I prefer not to or if it is a requirement, I will close this PR and include the change in the other PR. Main reason, writing these tests takes an awful lot of time and I already did it, I only created this PR to ease reviewing.

@ematipico ematipico removed their request for review August 24, 2022 10:44
@github-actions
Copy link

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.01    400.7±6.02ms     6.5 MB/sec    1.00    397.1±2.38ms     6.5 MB/sec
formatter/compiler.js                    1.00    247.9±2.30ms     4.2 MB/sec    1.01    250.0±2.12ms     4.2 MB/sec
formatter/d3.min.js                      1.00    200.1±2.10ms  1341.0 KB/sec    1.05    209.3±5.20ms  1282.3 KB/sec
formatter/dojo.js                        1.00     12.9±0.04ms     5.3 MB/sec    1.02     13.2±0.22ms     5.2 MB/sec
formatter/ios.d.ts                       1.02    261.9±2.47ms     7.1 MB/sec    1.00    257.2±1.52ms     7.3 MB/sec
formatter/jquery.min.js                  1.03     57.2±1.04ms  1478.8 KB/sec    1.00     55.4±1.96ms  1527.9 KB/sec
formatter/math.js                        1.03   441.6±11.08ms  1501.5 KB/sec    1.00    429.3±9.10ms  1544.6 KB/sec
formatter/parser.ts                      1.00      8.6±0.01ms     5.6 MB/sec    1.02      8.8±0.06ms     5.6 MB/sec
formatter/pixi.min.js                    1.01    241.1±6.53ms  1864.3 KB/sec    1.00    238.4±2.79ms  1885.4 KB/sec
formatter/react-dom.production.min.js    1.00     66.7±0.78ms  1767.5 KB/sec    1.01     67.6±1.53ms  1744.5 KB/sec
formatter/react.production.min.js        1.00      3.2±0.01ms  1948.3 KB/sec    1.01      3.3±0.05ms  1928.7 KB/sec
formatter/router.ts                      1.00      6.7±0.01ms     9.2 MB/sec    1.02      6.8±0.08ms     9.0 MB/sec
formatter/tex-chtml-full.js              1.03   566.3±13.39ms  1647.7 KB/sec    1.00   550.9±11.82ms  1693.8 KB/sec
formatter/three.min.js                   1.02    276.4±7.33ms     2.1 MB/sec    1.00    271.3±6.69ms     2.2 MB/sec
formatter/typescript.js                  1.02  1655.7±25.75ms     5.7 MB/sec    1.00   1622.9±6.68ms     5.9 MB/sec
formatter/vue.global.prod.js             1.00     90.3±2.74ms  1365.8 KB/sec    1.00     90.5±2.21ms  1364.0 KB/sec

@ematipico
Copy link
Contributor

ematipico commented Aug 24, 2022 •
edited
Loading

Would it be possible to have the unit test here?

I prefer not to or if it is a requirement, I will close this PR and include the change in the other PR. Main reason, writing these tests takes an awful lot of time and I already did it, I only created this PR to ease reviewing.

OK. I removed myself as reviewer - not comfortable enough to approve it. Feel free to merge it if you want or close it

@MichaReiser MichaReiser merged commit cc44a20 into main Aug 24, 2022
@MichaReiser MichaReiser deleted the perf/source-map-markers branch August 24, 2022 12:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
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
@MichaReiser @ematipico
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载