Fixes #994; poor left-recursive rule performance

[parrt]

No description provided.

[parrt]

Dear @antlr/antlr-targets folks, you should take a look at this small 60 line patch (method canDropLoopEntryEdgeInLeftRecursiveRule()) to ParserATNSimulator. It is a 10,000x speed boost. I added tests in PerformanceDescriptors that ignore all but Java target so you can unignore and see it take overnight to parse 10 lines. Then fix and it's fast :)

[parrt]

@hanjoes @pboyer looks like team antlr-targets invitation went to your spam buckets. can you check?

[hanjoes]

Yep, I've accepted the invitation.

[mike-lischke]

Hmm, strange, your largest test case from this patch takes just 3ms in the C++ target using a much more complex expr rule set with several left recursions (see the MySQL grammar), without this patch. So, it might not have the same impact on already fast language targets. I'll work on the translation soon.

[sharwell]

@mike-lischke this is an algorithm complexity change for the affected grammars. It only affects grammars and inputs with particular lookahead characteristics. For example, it affects the Java grammar yet no file in the entire JDK code base (12,000+ source files) hits the slow case. In other cases, the bad performance could be hit on a regular basis.

[mike-lischke]

I understand, but Ter mentioned that the test grammars would take minutes for him, while in the C++ target they only take ms (without the patch). Which is what makes me wondering. Ah, I didn't use the same grammar (even though I believe I have an expr rule which has the same characteristics and is even more complex).

[parrt]

Also the new tests that I added are set to be ignored for all but the Java target... just in case it looked like they were executing okay.

[mike-lischke]

Quite confusing what the actual patch is, when looking at this entry. I assume the thing that target author should care about is [fdee94d] (mentioned in the final merge)? But that's a bit more than 60 LOCs. @parrt btw, I can't find the C++ tests in runtime-tests/pom.xml file anymore. How are they now executed?

[ericvergnaud]

just did the Python migration it breaks 2 LR tests - it might take me some time to figure out why…

…

Le 25 nov. 2016 à 12:36, Terence Parr ***@***.***> a écrit : Dear @antlr/antlr-targets <https://github.com/orgs/antlr/teams/antlr-targets> folks, you should take a look at this small 60 line patch (method canDropLoopEntryEdgeInLeftRecursiveRule()) to ParserATNSimulator. It is a 10,000x speed boost. I added tests in PerformanceDescriptors that ignore all but Java target so you can unignore and see it take overnight to parse 10 lines. Then fix and it's fast :) — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#1404 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADLYJC7eKOgxLLxjFllf1HxQGkY8FVm0ks5rBmXrgaJpZM4K79mR>.

[parrt]

@ericvergnaud ok, lemme know if you need help

[parrt]

@mike-lischke You just have to implement function fdee94d#diff-49c5618cf842764cf250783fe7c0fe6cR1701 and inject a call to it elsewhere. the runtime tests work automatically.

[mike-lischke]

@parrt yes, yes, I was just wondering why I didn't see a full patch link, only those from individual commits. But this is probably the result of the already closed pull request. A Github usability issue...

[pboyer]

@parrt I don't see an invite for @antlr/antlr-targets in my email.

[parrt]

@pboyer i canceled and reinvited you. :) check spam

[parrt]

@mike-lischke yep, that's why i looked at "files changed" not "commits". much easier. :) Actually, i often pull into a branch and then run a diff on the two directories to see the complete set of changes.

[pboyer]

Thanks @parrt! I'm in. "10,000x"? Sounds like only a constant factor to me! 😆 I'll give this a shot, ASAP.