-
Notifications
You must be signed in to change notification settings - Fork 28
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
Catastrophic backtracking in function_definition
begin regex
#579
Comments
I'll test removing \G when I get a chance, although I'm pretty sure it will strongly effect correctness since it's not exactly a pattern that is just inserted casually. I tested removing it, and substituting it, and doing either breaks 80% of all tests. |
Can confirm Tho the lag seems to be constituted around variable declaration rather than the struct code
There is a double up of the comment handling group I'm curious I too also had performance issues with |
@jeff-hykin will the double up of the comment handling capture group be removed?
|
Hmm @RedCMD I don't see that pattern snippet in the file you linked.
Yes I think its worse, and the grammar generator should automatically convert |
sorry. the exact snippet has changed a bit better-cpp-syntax/autogenerated/cpp.tmLanguage.json Lines 2823 to 2824 in 562ec9a
better-cpp-syntax/autogenerated/cpp.tmLanguage.json Lines 4143 to 4144 in 562ec9a
better-cpp-syntax/autogenerated/cpp.tmLanguage.json Lines 7090 to 7091 in 562ec9a
|
ahhhhh..... @jeff-hykin
I see the problem now tho the issue is still present in the newer one |
Yeah, I should ask alexr if I can go ahead and delete
Alright @RedCMD , I think I found the source of the repeated pattern problem. Some composable patterns had |
still one left in better-cpp-syntax/autogenerated/cpp.tmLanguage.json Lines 3595 to 3596 in 190c42b
![]() ![]() the generator really likes putting |
Yeah, its cause of modularity. Code like We have the concept of "single_entity", which If this funciton were more intelligent, it would get rid of any unnecessary wrappings: I tried looking around for a generic regex minimizer/optimizer/uglifier a few years ago but didn't see anything of the sort. |
Actually, looking at the source there were some obvious mistakes. I updated the library, and now there's |
I know for a fact we optimized |
I can confirm the lag has been greatly reduced and with it being near instant for |
yep, same here. Looks like I can close this! |
Coming via microsoft/vscode#117264
The regex for
function_definition
takes >1s to evaluate at each step. This makes VS Code freeze for some minutes until it eventually recovers. If the regex would stop using\G
, then we would cache its search results and even if it would still be slow, at least the cost would be 1s in total for the entire line. As it is right now, the usage of\G
prohibits us from caching the search results, which means we need to re-search at each necessary offset (at each offset where tokenization advances).the regex
the test string
Here I have tested the regex at regex101.com and it leads to catastrophic backtracking:
![image](https://user-images.githubusercontent.com/5047891/142605984-d7fff3e1-5352-4c54-bb8a-955197740d95.png)
The text was updated successfully, but these errors were encountered: