More performance improvements

[jimidle]

Fix: Performance improvements for hashtable stuff and cache and fix to comparator

A lot of small changes in this one. Even though some of the commits look massive, they are all just
type changes from interface to struct pointers and similar things. They are all interrelated and cannot be separated
out.

This gets the Go runtime faster than Java on good grammars and within shouting distance on the terrible ones such as the MySql grammar. I am going to submit some improvements for those terrible grammars but my next task is memory pools for structs and related things that will have a big effect on these crummy grammars. This change gives me these results on my Mac for the MySql grammar:

Warmup run

Input	Elapsed parse time
analyze.sql	30.009214ms
bitrix_queries_cut.sql	123.628673491s
bitrix_queries_cut_small.sql	20.716369ms
case_sensitive_sql.sql	254.319312ms
ddl_alter.sql	257.266811ms
ddl_create.sql	4.042047644s
ddl_drop.sql	29.592294ms
ddl_flush.sql	13.866632ms
dml_delete.sql	188.947062ms
dml_insert.sql	1.054598045s
dml_replace.sql	103.230765ms
dml_select.sql	7.639615925s
dml_test_arithmetic_expression.sql	2.706622ms
dml_union.sql	205.721784ms
dml_update.sql	380.629755ms
dml_with.sql	81.843553ms
ext_tests.sql	1.329152887s
grant.sql	32.514802ms
kill.sql	8.69661ms
mysql_spec_comment.sql	52.157974ms
optimize.sql	3.617809ms
smoke_tests.sql	727.123175ms

Elapsed time (0): 1m40.088535933s

Performance round # 1

Input	Elapsed parse time
analyze.sql	5.172696ms
bitrix_queries_cut.sql	1m7.660708623s
bitrix_queries_cut_small.sql	20.4306ms
case_sensitive_sql.sql	108.66277ms
ddl_alter.sql	123.031896ms
ddl_create.sql	2.609207692s
ddl_drop.sql	26.253661ms
ddl_flush.sql	7.472161ms
dml_delete.sql	73.716928ms
dml_insert.sql	680.593347ms
dml_replace.sql	19.693085ms
dml_select.sql	4.64892433s
dml_test_arithmetic_expression.sql	1.329277ms
dml_union.sql	155.79117ms
dml_update.sql	232.733093ms
dml_with.sql	78.77783ms
ext_tests.sql	779.118564ms
grant.sql	14.549089ms
kill.sql	2.639832ms
mysql_spec_comment.sql	50.64345ms
optimize.sql	3.646558ms
smoke_tests.sql	743.777079ms

Elapsed time (1): 1m18.048023031s|

The bitrix_queries_cut.sql is still quite poor compared to Java. Its because the big input file causes gobs of allocations in both Java and Go. My memory changes will greatly affect these times.

@parrt PLease merge this when you can as I need these changes to make my memory pool changes. I will do some fancy branching and rebasing to deal with it until you can merge for me.

@kaby76 - you should find this is now runnable on your system. All benchmarks that are SLL or half decent grammars should really be very fast. There was an issue filed a while back with test source. The reporter wanted 0.2 seconds but we are now very very much faster than that with the go runtime.

[jimidle]

@parrt This PR is all ready to go mate! Phew!

[parrt]

This PR is all ready to go mate! Phew!
Hi! It looks like there are some merge conflicts according to GitHub

[jimidle]

@parrt The PR says it is ready to merge. Perhaps you saw an earlier email?

[jimidle]

Ok. I will check again. It’s saying ready to merge to dev. WTF?

…

On Tue, Mar 28, 2023 at 23:33 Terence Parr ***@***.***> wrote: This PR is all ready to go mate! Phew! Hi! It looks like there are some merge conflicts according to GitHub — Reply to this email directly, view it on GitHub <#4210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7TMDH2MXQ44ZCGFKTVPLW6MADPANCNFSM6AAAAAAWKGQZ3M> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[jimidle]

Now it says there are... GitHUb sometimes.

[jimidle]

Now it says there are not any.

[ericvergnaud]

No conflict here

[jimidle]

Nor me. I find some of the views tabs confusing. When you view changed files, it looks like there are conflicts, but they are all resolved and tested

…

On Wed, Mar 29, 2023 at 7:29 AM ericvergnaud ***@***.***> wrote: No conflict here — Reply to this email directly, view it on GitHub <#4210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7TMAVKMBJOTQYHD67KZLW6NX7DANCNFSM6AAAAAAWKGQZ3M> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[jimidle]

Looks good now @parrt

[parrt]

Unfortunately I've seen this before wear different people see different views. Check this out

[parrt]

merged manually using cmd-line. thanks, @jimidle !!!

[jimidle]

Yeah - I think this kind of thing happens when:

• I submit a PR from one branch
• That goes in fine
• I pull up to my dev branch in my fork-
• I rebase in to my other feature branch that I was working on
• Submit the PR from that branch and it all goes haywire.

It's because of forks I think. I don't have this issue using git flow and having people submit from a non-fork branch

Anyway -it's done, and I will avoid a rebase from dev if possible.

[AkbaraliShaikh]

Hello @jimidle,

When are we planning to merge these changes to the master branch?

between which jar should be used to generate the code using the dev branch?

cc: @parrt

[jimidle]

Hello @jimidle,

When are we planning to merge these changes to the master branch?

between which jar should be used to generate the code using the dev branch?

cc: @parrt

Soon I hope. It will happen when @parrt calls for a new release. My current work is basically preventing him from calling that. But I think it is worth the wait for the end of next week, when I will be done with all my experiments on memory allocation/pressure and add a few more performance tweaks that will be good for well formed grammars.

I have pretty much concluded the investigation into ways we might address memory and essentially I am finding that the existing GC is better than anything we can do in user code space. Mainly because it is extremely difficult to work out exactly when certain structs are no longer referenced and can be reused without getting the GC itself to tell me.

So, I think I am done with memory work as of right now. I may come back to this after the next release and see if I can work out exactly where and when I can reuse structs. But in this release I have added diagnostics and stats that allow at least me to work out what things are causing slow parsing - it is always a poor grammar, but it is useful to know what parts of a bad grammar cause difficulties, in case there can be a few wins.

I will leave it to Terence to decide when the new release should happen. It is not a ten minute task I think.

[AkbaraliShaikh]

Hello @jimidle,
When are we planning to merge these changes to the master branch?
between which jar should be used to generate the code using the dev branch?
cc: @parrt

Soon I hope. It will happen when @parrt calls for a new release. My current work is basically preventing him from calling that. But I think it is worth the wait for the end of next week, when I will be done with all my experiments on memory allocation/pressure and add a few more performance tweaks that will be good for well formed grammars.

I have pretty much concluded the investigation into ways we might address memory and essentially I am finding that the existing GC is better than anything we can do in user code space. Mainly because it is extremely difficult to work out exactly when certain structs are no longer referenced and can be reused without getting the GC itself to tell me.

So, I think I am done with memory work as of right now. I may come back to this after the next release and see if I can work out exactly where and when I can reuse structs. But in this release I have added diagnostics and stats that allow at least me to work out what things are causing slow parsing - it is always a poor grammar, but it is useful to know what parts of a bad grammar cause difficulties, in case there can be a few wins.

I will leave it to Terence to decide when the new release should happen. It is not a ten minute task I think.

Okay, Thank you.

Between, I have tried using the dev branch for benchmarking my rule paring code:

go get github.com/antlr/antlr4/runtime/Go/antlr/v4@dev

Used antlr-4.12.0-complete.jar to generate the code,
But after the code is generated, getting the below error in the *_parser.go

parser/myrule_parser.go:1367:67: not enough arguments in call to p.GetInterpreter().AdaptivePredict
        have (antlr.TokenStream, number, antlr.ParserRuleContext)
        want (*antlr.BaseParser, antlr.TokenStream, int, antlr.ParserRuleContext)

Anything I'm missing here?

[jimidle]

Codegen has changed. You need to use the dev version of the tool

…

On Sun, Apr 9, 2023 at 03:53 Akbar Shaikh ***@***.***> wrote: Hello @jimidle <https://github.com/jimidle>, When are we planning to merge these changes to the master branch? between which jar should be used to generate the code using the dev branch? cc: @parrt <https://github.com/parrt> Soon I hope. It will happen when @parrt <https://github.com/parrt> calls for a new release. My current work is basically preventing him from calling that. But I think it is worth the wait for the end of next week, when I will be done with all my experiments on memory allocation/pressure and add a few more performance tweaks that will be good for well formed grammars. I have pretty much concluded the investigation into ways we might address memory and essentially I am finding that the existing GC is better than anything we can do in user code space. Mainly because it is extremely difficult to work out exactly when certain structs are no longer referenced and can be reused without getting the GC itself to tell me. So, I think I am done with memory work as of right now. I may come back to this after the next release and see if I can work out exactly where and when I can reuse structs. But in this release I have added diagnostics and stats that allow at least me to work out what things are causing slow parsing - it is always a poor grammar, but it is useful to know what parts of a bad grammar cause difficulties, in case there can be a few wins. I will leave it to Terence to decide when the new release should happen. It is not a ten minute task I think. Okay, Thank you. Between, I have tried using the dev branch for benchmarking my rule paring code: go get ***@***.*** Used antlr-4.12.0-complete.jar to generate the code, But after the code is generated, getting the below error in the *_parser.go parser/myrule_parser.go:1367:67: not enough arguments in call to p.GetInterpreter().AdaptivePredict have (antlr.TokenStream, number, antlr.ParserRuleContext) want (*antlr.BaseParser, antlr.TokenStream, int, antlr.ParserRuleContext) Anything I'm missing here? — Reply to this email directly, view it on GitHub <#4210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7TMCRWRMH6DBTB4I3UHDXAG647ANCNFSM6AAAAAAWKGQZ3M> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ericvergnaud]

Hi, you need to build it, I’m afraid...

…

Le 11 avr. 2023 à 09:38, Akbar Shaikh ***@***.***> a écrit : Codegen has changed. You need to use the dev version of the tool … <x-msg://17/#> On Sun, Apr 9, 2023 at 03:53 Akbar Shaikh @.> wrote: Hello @jimidle <https://github.com/jimidle> https://github.com/jimidle, When are we planning to merge these changes to the master branch? between which jar should be used to generate the code using the dev branch? cc: @parrt <https://github.com/parrt> https://github.com/parrt Soon I hope. It will happen when @parrt <https://github.com/parrt> https://github.com/parrt calls for a new release. My current work is basically preventing him from calling that. But I think it is worth the wait for the end of next week, when I will be done with all my experiments on memory allocation/pressure and add a few more performance tweaks that will be good for well formed grammars. I have pretty much concluded the investigation into ways we might address memory and essentially I am finding that the existing GC is better than anything we can do in user code space. Mainly because it is extremely difficult to work out exactly when certain structs are no longer referenced and can be reused without getting the GC itself to tell me. So, I think I am done with memory work as of right now. I may come back to this after the next release and see if I can work out exactly where and when I can reuse structs. But in this release I have added diagnostics and stats that allow at least me to work out what things are causing slow parsing - it is always a poor grammar, but it is useful to know what parts of a bad grammar cause difficulties, in case there can be a few wins. I will leave it to Terence to decide when the new release should happen. It is not a ten minute task I think. Okay, Thank you. Between, I have tried using the dev branch for benchmarking my rule paring code: go get @. Used antlr-4.12.0-complete.jar to generate the code, But after the code is generated, getting the below error in the _parser.go parser/myrule_parser.go:1367:67: not enough arguments in call to p.GetInterpreter().AdaptivePredict have (antlr.TokenStream, number, antlr.ParserRuleContext) want (antlr.BaseParser, antlr.TokenStream, int, antlr.ParserRuleContext) Anything I'm missing here? — Reply to this email directly, view it on GitHub <#4210 (comment) <#4210 (comment)>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ7TMCRWRMH6DBTB4I3UHDXAG647ANCNFSM6AAAAAAWKGQZ3M . You are receiving this because you were mentioned.Message ID: @.*> Sorry, Not able to find it, Can you please help me here? Where can I find the dev version of tool jar? — Reply to this email directly, view it on GitHub <#4210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZNQJBKESYPUFHUPVRWUS3XAUC6ZANCNFSM6AAAAAAWKGQZ3M>. You are receiving this because you commented.

[jimidle]

Hi, you need to build it, I’m afraid...

Actually you don't I guess. When the build checks run for the PR request, it does actually upload the built jar. But it is a little convoluted.

For this PR:

• https://github.com/antlr/antlr4/actions/runs/4663531339
• You can see that there are artifacts for the Tool. Because it is Java, you can download any of the _tool outputs.
• You will get a zip download
• Expand the Zip
• Find the ANTLR tool directory, then look under the target directory and you will find the snapshot complete jar.

[AkbaraliShaikh]

@jimidle Here are the perf results for the master vs dev branch

Master Branch

go test -run=Bench -benchmem -bench=. -benchtime=60s
goos: darwin
goarch: arm64
pkg: source.golabs.io/asgard/spikes/rule-engine
BenchmarkParseRule-10                            4565505             16049 ns/op           12536 B/op        259 allocs/op
PASS
ok      source.golabs.io/asgard/spikes/rule-engine      222.331s

Dev Branch

go test -run=Bench -benchmem -bench=. -benchtime=60s
goos: darwin
goarch: arm64
pkg: source.golabs.io/asgard/spikes/rule-engine
BenchmarkParseRule-10                            4908742             15095 ns/op           12192 B/op        230 allocs/op
PASS
ok      source.golabs.io/asgard/spikes/rule-engine      218.251s

[jimidle]

I would need to see: - Your benchmark code - Your grammar - Your input In order to comment on this. Did you already post that somewhere? Also, is this SLL mode, SLL first then LL? Jim

…

On Tue, Apr 11, 2023 at 6:39 PM Akbar Shaikh ***@***.***> wrote: @jimidle <https://github.com/jimidle> Here are the perf results for the master vs dev branch *Master Branch* BenchmarkParseRule-10 6449614 20035 ns/op 8882 B/op 179 allocs/op BenchmarkEvaluateUsingAntlrVisitor-10 74199505 1143 ns/op 208 B/op 12 allocs/op *Dev Branch* BenchmarkParseRule-10 3132622 23037 ns/op 12192 B/op 230 allocs/op BenchmarkEvaluateUsingAntlrVisitor-10 79340918 881.7 ns/op 160 B/op 9 allocs/op — Reply to this email directly, view it on GitHub <#4210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7TMEIWQXVAZ5PGDH3PCTXAUYF3ANCNFSM6AAAAAAWKGQZ3M> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[AkbaraliShaikh]

@jimidle there was some mistake, I have updated the above results

@jimidle here are the above requested details which are used perf testing: https://github.com/AkbaraliShaikh/test

[jimidle]

Ok cool. If you want to send the benchmark to me privately, then that’s ok with me Also, there is an outstanding PR that improves good grammars. And one I’m working on that allows a single thread mode to avoid muteness. That’s a bit experimental, so you have to use a build configuration tag for it.

…

On Tue, Apr 11, 2023 at 19:03 Akbar Shaikh ***@***.***> wrote: @jimidle <https://github.com/jimidle> there was some mistake, I have update the above results — Reply to this email directly, view it on GitHub <#4210 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7TMHL2I2WJMPDQYC7TX3XAU3BXANCNFSM6AAAAAAWKGQZ3M> . You are receiving this because you were mentioned.Message ID: ***@***.***>