perf(linter): Use aho-corasick instead of regex for default comment patterns in no-fallthrough#5901
Conversation
… `no-fallthrough`
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @camchenry and the rest of your teammates on |
camchenry
changed the title
aho-corasick instead of regex for default comment patterns in no-fallthroughaho-corasick instead of regex for default comment patterns in no-fallthrough
| default_comment_pattern: AhoCorasick::builder() | ||
| .ascii_case_insensitive(true) | ||
| .build(DEFAULT_FALLTHROUGH_COMMENT_PATTERNS) | ||
| .expect("Could not build AhoCorasick for default comment patterns"), |
There was a problem hiding this comment.
Can we provide the name of the rule so users know where and how to fix their configs?
CodSpeed Performance ReportMerging #5901 will not alter performanceComparing Summary
|
|
I tested this locally and it seems like this might not actually be much of a perf win. Seems like it is 2-3% slower on the large benchmarks like checker.ts and cal.com, but faster in small files (~6-7%), but not at a statistically significant level. |
|
This PR could still be useful if it lets us remove our dependency on |
|
@DonIsaac I think we still have to depend on regex still unfortunately, because the custom comment pattern config is allowed to be any pattern. |
2 participants
theory: using Regex is slow in most cases, since we have a fixed number of patterns that we need to search for. but we do still need to support it since the user can pass a custom regex pattern. however, we can at least try to use a simpler
aho-corasickmatcher for the default strings, which should likely be the majority of cases.