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

refactor: replace fastStartsWith & fastStartsWithFrom #4518

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Dec 11, 2024

Fixes #4517.

Per benchmarks mentioned in the issue, fastStartsWith & fastStartsWithFrom are consistently slower on modern Chrome and modern Firefox (up to 0.5x to 1x slower).

All tests and lints passed locally on my machine.

@seia-soto
Copy link
Member

Hi @SukkaW ,

Thanks for the contribution. We're measuring the actual speed difference in multiple platforms to be sure. Once we're ready, we'll merge it.

Best

@seia-soto seia-soto added the PR: Performance 🏃‍♀️ Increment minor version when merged label Dec 12, 2024
@seia-soto seia-soto self-requested a review December 12, 2024 07:47
@seia-soto seia-soto self-assigned this Dec 25, 2024
@chrmod
Copy link
Member

chrmod commented Jan 23, 2025

It took a while to make some benchmarks but we are now sure the changes are beneficial. Thank you very much for your contribution!

@chrmod chrmod merged commit a1cdd40 into ghostery:master Jan 23, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Performance 🏃‍♀️ Increment minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fastStartsWith is no longer faster than the native String.prototype.startsWith
4 participants