-
Notifications
You must be signed in to change notification settings - Fork 192
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
Switch to using thread_local regular expressions to stop mutext contention #996
Conversation
f215978
to
77269e7
Compare
Codecov ReportPatch and project coverage have no change.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #996 +/- ##
=======================================
Coverage 90.93% 90.93%
=======================================
Files 254 254
Lines 25972 25972
=======================================
Hits 23617 23617
Misses 2355 2355 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This looks great to me. Happy to merge once #995 is in.
Oh, one thing though: you'll have to update the MSRV to (looks like 1.70), but that should be fine. |
Done @zsol! I also bumped some dependencies. notably the |
ab536ea
to
4ce22ba
Compare
Thank you! |
Fixes #986. This is also rebased on top of #995 to allow me to run the benchmarks.
Summary
When calling
libcst_native::parse_module
from multiple threads the parsing performance drops dramatically. This is somewhat documented in the regex_automata crate and in other places around the internet:Benchmarks
I've added a multi-threaded benchmark using Rayon. The use of thread locals improves the throughput by 22% and reduces the time by 18%:
However the throughput increase I've seen with high-volume "real-world" parsing is much, much higher.
Benchmarks
These are the results of the benchmark added in this change, with and without the
thread_local!
change. It charts the time taken to parse the fixture files. The number of files is multiplied by the number of threads, which ranges from 1 to 10. At 10 threads the average time taken is 50% faster.Benchmark: Before
Benchmark: Before