Skip to content
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

implement faster HashMap #4551

Merged
merged 1 commit into from
Mar 11, 2024
Merged

implement faster HashMap #4551

merged 1 commit into from
Mar 11, 2024

Conversation

ericvergnaud
Copy link
Contributor

new faster HashMap

@ericvergnaud
Copy link
Contributor Author

ignore unrelated php failure

@ericvergnaud ericvergnaud requested a review from kaby76 March 8, 2024 20:27
@ericvergnaud ericvergnaud changed the title implement faster HashMap and test spec implement faster HashMap Mar 8, 2024
@kaby76
Copy link
Contributor

kaby76 commented Mar 10, 2024

For this PR, there is a 40% speed up. (2.8537 +- SD 0.0124 vs 2.0492 +- SD 0.033).

java

@kaby76
Copy link
Contributor

kaby76 commented Mar 10, 2024

(There may be other optimizations that Antlr4ng does, but I haven't gotten that far.)

@ericvergnaud
Copy link
Contributor Author

ericvergnaud commented Mar 10, 2024 via email

@kaby76
Copy link
Contributor

kaby76 commented Mar 10, 2024

Yes, something like ~60%. There are probably other optimizations to look at.

At some point, can we get a release of 4.13.2?? No hurries though, after other fixes are considered. I did not like this 4.13.1-patch1 release because that breaks my testing code, and I don't think GitHub Dependabot even considers this a release.

@kaby76
Copy link
Contributor

kaby76 commented Mar 10, 2024

This PR passes all grammars-v4 tests.

@ericvergnaud
Copy link
Contributor Author

At some point, can we get a release of 4.13.2?? No hurries though, after other fixes are considered. I did not like this 4.13.1-patch1 release because that breaks my testing code, and I don't think GitHub Dependabot even considers this a release.

That's a question for @parrt.

@ericvergnaud ericvergnaud merged commit 69cfd8e into antlr:dev Mar 11, 2024
41 of 42 checks passed
@ericvergnaud ericvergnaud deleted the new-hash-map branch March 11, 2024 10:15
@ericvergnaud
Copy link
Contributor Author

@kaby76 it's be interesting to run your performance benchmark on the merged dev version

@kaby76
Copy link
Contributor

kaby76 commented Mar 11, 2024

The runtime for the tip of dev is better.

java

But, it is still missing something as Antlr4ng is faster. I am still surprised the hash value caching didn't help. I must have done something wrong, or it should have been applied somewhere else. Looking....

@ericvergnaud
Copy link
Contributor Author

antlr4ng also implements specialized constructors which avoid testing instance types. It might be worth doing this for the objects that are instantiated millions of times (such as ATNConfig). But we might be reaching a point where profiling becomes necessary to better understand the remaining bottlenecks.
Sam mentioned that the reason for not caching ATNConfig hashCode is memory footprint.

@ericvergnaud
Copy link
Contributor Author

But, it is still missing something as Antlr4ng is faster. I am still surprised the hash value caching didn't help. I must have done something wrong, or it should have been applied somewhere else. Looking....

I can revise and rebase that branch if you're willing to double-check that...

@kaby76
Copy link
Contributor

kaby76 commented Mar 11, 2024

I'm profiling the CSharp target because your PR changed one file in that target. (IMO, C# has far-superior profiling tools to anything I've seen in all other targets.)

@parrt parrt added this to the 4.13.2 milestone Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants