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(parser): inline identifier handler into byte handlers #2416

Conversation

overlookmotel
Copy link
Collaborator

@overlookmotel overlookmotel commented Feb 15, 2024

Inlining identifier_name_handler into the byte handlers for a-z, A-Z, _ and $ produces a modest performance boost, as it's a hot path.

@overlookmotel
Copy link
Collaborator Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@github-actions github-actions bot added the A-parser Area - Parser label Feb 15, 2024
@overlookmotel
Copy link
Collaborator Author

Will have to check if this bloats binary size too much. It may not be worth it.

How do I go about that? I was expecting the bloat CI job to run, but just realised it's not triggered on PRs (and maybe isn't for that purpose anyway).

Copy link

codspeed-hq bot commented Feb 15, 2024

CodSpeed Performance Report

Merging #2416 will not alter performance

Comparing 02-15-perf_parser_inline_identifier_handler_into_byte_handlers (ce74089) with main (3ff3495)

Summary

✅ 27 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Feb 16, 2024

but just realised it's not triggered on PRs (and maybe isn't for that purpose anyway).

You need to click the top right "run workflow" from the UI https://github.com/oxc-project/oxc/actions/workflows/bloat.yml

It'll display the result on the action page once it's done.

e.g. https://github.com/oxc-project/oxc/actions/runs/7930840687

@Boshen
Copy link
Member

Boshen commented Feb 16, 2024

I think this optimization is a bit overkill if you look at the absolute change.

@Boshen Boshen closed this Feb 16, 2024
@overlookmotel
Copy link
Collaborator Author

Fair enough!

It might be worth revisiting once SIMD is implemented, as then identifier_name_handler will shrink considerably and cost of inlining it would drop, but that's a way off...

@Boshen Boshen deleted the 02-15-perf_parser_inline_identifier_handler_into_byte_handlers branch March 25, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area - Parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants