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

Use memchr to optimize get_text_slice #344

Merged
merged 1 commit into from
May 5, 2024

Conversation

clubby789
Copy link
Contributor

Running the benchmarks on my machine, I get these results:

parse_ctx_runtime/browser                                                                            
                        time:   [106.61 µs 106.73 µs 106.85 µs]
                        change: [-17.886% -17.720% -17.552%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  8 (8.00%) high mild
  1 (1.00%) high severe
parse_ctx_runtime/preferences                                                                            
                        time:   [277.45 µs 277.83 µs 278.29 µs]
                        change: [-8.5355% -8.3601% -8.1949%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

With some stress tests against Fluent files used in rustc, I saw improvements of 20-30%.

@clubby789
Copy link
Contributor Author

Some notable improvements when used in rustc:

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.3% [-0.3%, -0.2%] 3
Improvements ✅
(secondary)
-1.5% [-4.1%, -0.3%] 41
All ❌✅ (primary) -0.3% [-0.3%, -0.2%] 3

@waywardmonkeys
Copy link
Collaborator

@flodolo You might want to approve the CI runs for this.

Copy link
Collaborator

@alerque alerque left a comment

Choose a reason for hiding this comment

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

This seems reasonable. I think we're mostly waiting on getting a safe-harbor release out, then we can start moving forward with improvements. Thanks for the contribution.

@alerque alerque merged commit 2aa38ae into projectfluent:main May 5, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants