Skip to content

improve sourcemap performance#1881

Merged
ai merged 3 commits intopostcss:mainfrom
romainmenke:improve-sourcemap-performance--philosophical-spiny-dogfish-3eb029c1c8
Sep 18, 2023
Merged

improve sourcemap performance#1881
ai merged 3 commits intopostcss:mainfrom
romainmenke:improve-sourcemap-performance--philosophical-spiny-dogfish-3eb029c1c8

Conversation

@romainmenke
Copy link
Copy Markdown
Contributor

@romainmenke romainmenke commented Sep 16, 2023 •
edited
Loading

related change to PostCSS benchmarks : postcss/benchmark#32

romainmenke
romainmenke commented Sep 16, 2023
View reviewed changes
lib/map-generator.js Outdated
Comment on lines +21 to +23
this.memoizedFileURLs = new Map();
this.memoizedPaths = new Map();
this.memoizedURLs = new Map();
Copy link
Copy Markdown
Contributor Author

@romainmenke romainmenke Sep 16, 2023

Choose a reason for hiding this comment

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

Specific operations were done very frequently and were slow.
As far as I can tell these can be memoized.
This dramatically improves performance of sourcemap generation.

I don't have a strong opinion on naming or best practices here.
Please advice if there is a more idiomatic way to memoize these values within PostCSS.

@ai
Copy link
Copy Markdown
Member

ai commented Sep 16, 2023

Can you show the benchmark results (before/after) on you machine?

@romainmenke
Copy link
Copy Markdown
Contributor Author

romainmenke commented Sep 16, 2023

On my machine (M2 air) :

[22:24:27] Running suite Sourcemaps [/Users/romainmenke/projects/postcss-benchmark/sourcemaps.js]...
[22:24:33]    PostCSS x 8.41 ops/sec ±2.61% (45 runs sampled)
[22:24:39]    Next PostCSS x 17.42 ops/sec ±3.29% (85 runs sampled)
[22:24:39] Fastest test is Next PostCSS at 2.1x faster than PostCSS

Next PostCSS: 57 ms  (2.1 times faster)
PostCSS:      119 ms

ai
ai reviewed Sep 16, 2023
View reviewed changes
ai
ai reviewed Sep 16, 2023
View reviewed changes
lib/map-generator.js Outdated
file = relative(from, file)
return file
let path = relative(from, file)
this.memoizedPaths.set(file, path);
Copy link
Copy Markdown
Member

@ai ai Sep 16, 2023

Choose a reason for hiding this comment

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

;

romainmenke and others added 2 commits September 16, 2023 22:41
@romainmenke @ai
Update lib/map-generator.js
efa442c 
Co-authored-by: Andrey Sitnik <andrey@sitnik.ru>
@romainmenke
apply suggestions from code review
6a291d6
@ai
Copy link
Copy Markdown
Member

ai commented Sep 16, 2023

Looks amazing. I will accept it tomorrow. Don’t want to accept and release before going to bed :D.

@ai ai merged commit b2be58a into postcss:main Sep 18, 2023
@romainmenke romainmenke deleted the improve-sourcemap-performance--philosophical-spiny-dogfish-3eb029c1c8 branch September 18, 2023 21:21
@ai
Copy link
Copy Markdown
Member

ai commented Sep 18, 2023

Thanks. Released in 8.4.30.

@romainmenke
Copy link
Copy Markdown
Contributor Author

romainmenke commented Sep 18, 2023

Awesome! thank you 🙇

@sync-by-unito sync-by-unito bot mentioned this pull request Jan 31, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ai ai ai left review comments

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

@romainmenke @ai