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

antlr4 jar doubled in size in 4.9.3 #4226

Closed
mikolajpod opened this issue Apr 12, 2023 · 7 comments
Closed

antlr4 jar doubled in size in 4.9.3 #4226

mikolajpod opened this issue Apr 12, 2023 · 7 comments

Comments

@mikolajpod
Copy link

File antlr4-4.9.3.jar is twice as big as antlr4-4.9.2.jar (2 640 255 vs 1 231 617 bytes).
Using 7-zip I can track difference to org\antlr\v4\unicode directory.
Using JD-GUI (Java decompiler) I can see, that in 4.9.3 there are additional wrapper classes which probably increase file size.

7z_492
7z_493

@ericvergnaud
Copy link
Contributor

Yes that's because we had to incorporate icu. Is this an issue ?

@mikolajpod
Copy link
Author

mikolajpod commented Apr 13, 2023

Yes that's because we had to incorporate icu.

This is not because of ICU. This is because of template change. Just look at commit d730364 with message
"Upgrade com.ibm.icu:icu4j to latest (69.1) to avoid potential vulnerabilities it brings Also change the template to suite this upgrade, as this upgrade makes the java class larger than jvm can accept." dated at 25.08.2021 17:34.

It changes file unicodedata.st so that now each method is wrapped in separate static class. Class size limit is avoided but with some consequences. I understand, where it comes from and also I understand this was simple and quick resolution of class size limit problem. But maybe it can be done other way, e.g. dividing methods into 2 (or 3 or even 10) classes instead of 1000+

Is this an issue ?

Think about all those copies transferred trough Internet and lying on hard drives :)

Best regards,
Mikołaj

@KvanTTT
Copy link
Member

KvanTTT commented Apr 13, 2023

This looks like a problem.

The size should be decreased to the previous value if it's possible.

@KvanTTT
Copy link
Member

KvanTTT commented Apr 14, 2023

I understand, where it comes from and also I understand this was simple and quick resolution of class size limit problem. But maybe it can be done other way, e.g. dividing methods into 2 (or 3 or even 10) classes instead of 1000+

You are right, this can be fixed in a much better way. I'll bring the solution ASAP.

@KvanTTT
Copy link
Member

KvanTTT commented Apr 14, 2023

I've fixed it in referenced PR. FYI, update to the new vesion also increases the size, but generator optimization values more.

@mikolajpod
Copy link
Author

@KvanTTT Thank you.

@KvanTTT
Copy link
Member

KvanTTT commented May 18, 2023

@parrt it can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants