-
Notifications
You must be signed in to change notification settings - Fork 37
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
org.jetbrains.org.objectweb.asm.MethodTooLargeException: Method too large #252
Comments
I can see that protected fun writeMessageDescriptor(type: File.Type.Message) {
val allFields = type.sortedStandardFieldsWithOneOfs()
val chunkSize = 200 // This should be configurable.
val needToChunk = allFields.size > chunkSize This should be adjusted or exposed as configurable property. Wdyt @garyp? |
@sliskiCode what would be the benefit of making it configurable? In other words, in what conditions would you use a different chunk size? |
So benefit is simple. Fixed size of 200 is not good enough. It works for a golden path case but does not when oneof cases hold some metadata. In my example oneof has 132 cases + metadata and generated descriptor exceeds JVM method limit. I am not sure if this should be configurable but at least tweaked for more complex cases. Maybe 100 would be a good value candidate 🤔. |
Sorry for letting this sit for over a year 🙈 Let's try reducing the chunk threshold to 100 and see if that's good enough in practice. If it isn't, we can investigate a more complex heuristic. I'll try to get this change added in the upcoming release. |
The previous size of 200 worked fine when a message contained lots of fields without any custom options. But custom options on a field increase the size of the code that is generated for the field's field descriptor. Which means we can fit fewer field descriptor constructor calls in a single method before the method's bytecode becomes too big and generates a `MethodTooLargeException`. For now we'll just reduce the chunking size from 200 to 100 to give us some more breathing room. Sufficiently large and complex protobuf message definitions can still lead to an exception though. If that happens often then we can implement further improvements to the chunking. Fixes #252
The previous size of 200 worked fine when a message contained lots of fields without any custom options. But custom options on a field increase the size of the code that is generated for the field's field descriptor. Which means we can fit fewer field descriptor constructor calls in a single method before the method's bytecode becomes too big and generates a `MethodTooLargeException`. For now we'll just reduce the chunking size from 200 to 100 to give us some more breathing room. Sufficiently large and complex protobuf message definitions can still lead to an exception though. If that happens often then we can implement further improvements to the chunking. Fixes #252
I've reduced the chunk size from 200 to 100. I'm sure we can come up with a degenerate test case that will still fail even with a chunk size of 100. I don't know if there's any real-word protobuf usage that would do this though. If pbandk users do still run into this issue even with the reduced chunk size, then I can see two future paths forward:
|
The previous size of 200 worked fine when a message contained lots of fields without any custom options. But custom options on a field increase the size of the code that is generated for the field's field descriptor. Which means we can fit fewer field descriptor constructor calls in a single method before the method's bytecode becomes too big and generates a `MethodTooLargeException`. For now we'll just reduce the chunking size from 200 to 100 to give us some more breathing room. Sufficiently large and complex protobuf message definitions can still lead to an exception though. If that happens often then we can implement further improvements to the chunking. Fixes #252
Hi there,
I have discovered that codegen breaks when
message
has to many cases inoneof {}
block. It looks like generated function underdescriptor
is too large for JVM byte code. Function could be sliced in a way that everyadd()
list invocation is a separate method. Wdyt?Can be reproduce by writing UT under:
pbandk.gen.CodeGeneratorTest
test_big_oneof.proto
The text was updated successfully, but these errors were encountered: