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

perf(css/ast): Reduce token size #6569

Merged
merged 43 commits into from Dec 6, 2022
Merged

Conversation

alexander-akait
Copy link
Collaborator

@alexander-akait alexander-akait commented Dec 2, 2022

Description:

This is one of the performance improvements PR

Gnuplot not found, using plotters backend
css/lexer/bootstrap_5_1_3                                                                             
                        time:   [5.0974 ms 5.1213 ms 5.1480 ms]
                        change: [-11.146% -10.572% -10.024%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) high mild
  6 (6.00%) high severe

css/lexer/foundation_6_7_4                                                                             
                        time:   [4.2814 ms 4.2999 ms 4.3219 ms]
                        change: [-6.3111% -4.5331% -2.8664%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) high mild
  7 (7.00%) high severe

css/lexer/tailwind_3_1_1                                                                            
                        time:   [802.97 us 807.05 us 812.42 us]
                        change: [-10.144% -7.5341% -4.9534%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  7 (7.00%) high severe

Parser was improved too

BREAKING CHANGE:

Yes

Related issue (if exists):

No

@alexander-akait
Copy link
Collaborator Author

alexander-akait commented Dec 2, 2022

I think we can box more things

@alexander-akait alexander-akait marked this pull request as draft December 2, 2022 14:22
@kdy1 kdy1 self-assigned this Dec 3, 2022
@kdy1 kdy1 added this to the Planned milestone Dec 3, 2022
@alexander-akait alexander-akait marked this pull request as ready for review December 4, 2022 23:15
deserialize = "__D: rkyv::de::SharedDeserializeRegistry"
))
)]
pub enum Token {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is too much boxing.
Are you trying to optimive memmove?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried different things, lexer is faster when we box tokens, because we use them in AST, i.e. ComponentValues::PreservedTokens, so memory usage was improved good, parser performance unfortunately was not very improved

Copy link
Member

@kdy1 kdy1 Dec 5, 2022

Choose a reason for hiding this comment

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

Edit: We need to box Url, Dimension, BadUrl

@kdy1 kdy1 changed the title fix(css/parser): reduce token size perf(css/ast): Reduce token size Dec 6, 2022
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Thanks!


swc-bump:

  • swc_css_ast --breaking

@kdy1 kdy1 enabled auto-merge (squash) December 6, 2022 01:24
Copy link
Collaborator

@swc-bot swc-bot left a comment

Choose a reason for hiding this comment

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

Automated review comment generated by auto-rebase script

@kdy1 kdy1 merged commit 8633d27 into swc-project:main Dec 6, 2022
@alexander-akait alexander-akait deleted the fix-css-perf branch December 6, 2022 02:00
@kdy1 kdy1 modified the milestones: Planned, v1.3.22 Dec 9, 2022
@swc-project swc-project locked as resolved and limited conversation to collaborators Jan 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants