gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.51k stars 373 forks source link

Fix error initializeEnumMap() is exceeding the 65535 bytes limit #9954

Closed pmskintner closed 2 months ago

pmskintner commented 4 months ago

During build we hit a the error

The code of method initializeEnumMap() is exceeding the 65535 bytes limit.

This gets around the error by splitting the method into as many methods as needed.

pmskintner commented 4 months ago

Thanks for the contribution - keep in mind that this is going to eventually fail as well, as there is an upper limit to the size of a class file too. I don't have a concrete suggestion here, but it could be worth considering while you're making the change.

It might be worth it to change the implementation to not produce a Arrays.asList(MyEnum.FIRST, MyEnum.SECOND, MyEnum.THIRD) etc, but instead to use Arrays.asList(MyEnum.values()) - that would also allow the implementation to scale with the number of enum types rather than with the total number of enum values in all types.

Worth making either change to further improve this?

It's not infinite but trying to figure out what the limit the upper limit on a class file is, it looks like the next limitation we'd run into having 65535 methods in one class. That's very far away for our use case as we're on only on the 3rd initializeEnumMap method with this change.

To your second question, I can't tell where you're talking about can you point it out please. Did you mean the line:

sb.append(String.format("%s.<%s<?>> asList(", Arrays.class.getCanonicalName(), Enum.class .getCanonicalName()));

This line is generating the string mapping for enums that share the same name, so is generating something like Arrays.asList(MyFirstEnum.FOO, MySecondEnum.FOO, MyThirdEnum.FOO)

niloc132 commented 4 months ago

It's not infinite but trying to figure out what the limit the upper limit on a class file is, it looks like the next limitation we'd run into having 65535 methods in one class. That's very far away for our use case as we're on only on the 3rd initializeEnumMap method with this change. That makes sense - I thought there was another size limitation we might bump into, but didn't remember where it was. We've hit limitations like this before in generated code (last time was for gwt-rpc I believe), so just wanted to be sure that you had looked ahead to any other surprises.

This line is generating the string mapping for enums that share the same name, so is generating something like Arrays.asList(MyFirstEnum.FOO, MySecondEnum.FOO, MyThirdEnum.FOO)

Ah you're right of course, thank you for clarifying what I didn't read correctly.

niloc132 commented 4 months ago

I merged locally to main and ran AutoBeanSuite, RequestFactorySuite, and RequestFactoryEditorSuite successfully, but would like to be sure that local builds merged up to date worked for you @pmskintner.

pmskintner commented 4 months ago

I merged gwt main into our fork locally and ran the test suites you mentioned, here are the reports:

TEST-com.google.web.bindery.requestfactory.gwt.RequestFactoryEditorSuite.txt TEST-com.google.web.bindery.requestfactory.gwt.RequestFactoryEditorSuite.txt TEST-com.google.web.bindery.autobean.AutoBeanSuite.txt

The upcoming 2.12 release would be great to get into, is that going to be tagged off the current main?

niloc132 commented 4 months ago

Yes, that will be cut from main, once we have the other Java 17 features in. Can you merge this up to date with main for a quick round of your own testing of the feature, just to be sure, rather than rely on git doing it for us?

pmskintner commented 3 months ago

We merged main and validated locally.