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: faster resetIconPosition avoiding to parse path twice #10497

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

mondeja
Copy link
Contributor

@mondeja mondeja commented Sep 3, 2024

This is a continuation of #10488.

Just seen that PR and realized that the parsing of the path (d) is done twice. One in svgpath(path) and another in svgPathBbox(path) as svgPathBbox uses svgpath internally.

So I released a new version of svg-path-bbox that also accepts SvgPath interfaces to avoid the double parsing in situations like this. Instead of parsing again, svgpath clones the segments and is so much faster.

Copy link
Contributor

github-actions bot commented Sep 3, 2024

Warnings
⚠️

This PR modified helper functions in lib/ but not accompanying tests.
That's okay so long as it's refactoring existing code.

Messages
📖 ✨ Thanks for your contribution to Shields, @mondeja!

Generated by 🚫 dangerJS against 6c97e1d

@LitoMore
Copy link
Contributor

LitoMore commented Sep 4, 2024

I just tested this on my machine, it optimized my process from 60 ms to 38 ms.

Top 10 slow icons:
┌───────┬───────────────────────────┬───────────┐
│ (idx) │ title                     │ time (ms) │
├───────┼───────────────────────────┼───────────┤
│     0 │ "Lerna"                   │        38 │
│     1 │ "PUBG"                    │        16 │
│     2 │ "Composer"                │         6 │
│     3 │ "Elsevier"                │         6 │
│     4 │ "Gutenberg"               │         4 │
│     5 │ "New Japan Pro-Wrestling" │         4 │
│     6 │ "Porsche"                 │         4 │
│     7 │ "Unilever"                │         4 │
│     8 │ "1001Tracklists"          │         2 │
│     9 │ "1Password"               │         2 │
└───────┴───────────────────────────┴───────────┘

@chris48s chris48s added frontend The React app and the infrastructure that supports it core Server, BaseService, GitHub auth and removed frontend The React app and the infrastructure that supports it labels Sep 4, 2024
Copy link
Contributor

github-actions bot commented Sep 4, 2024

🚀 Updated review app: https://pr-10497-badges-shields.fly.dev

@chris48s chris48s added this pull request to the merge queue Sep 4, 2024
Merged via the queue into badges:master with commit f776f81 Sep 4, 2024
24 checks passed
@mondeja mondeja deleted the logoSize-more-perf branch September 4, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Server, BaseService, GitHub auth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants