Fix java compilation issues when processing large .proto files #101
Conversation
Fix issues 579 and 501 on the code.google.com issues list. Specifically, large .proto files lead to too much static code, leading to a compilation error from javac: "code too large". This divides the code used in static initialization into multiple methods to avoid that error. Also, this incorporates the fix in issue 501 on the code.google.com issues list to call registry.add only once per extension.
|
Is this patch originally from daniel.martin@crowdstrike.com? If yes, we'll need him to sign Google CLA as well before we can accept this patch. |
|
Oh crud, I hadn't meant to update git with my work address. Yes, that's me too - I'll go sign the same CLA with my work account too. |
|
CLAs look good, thanks! |
|
@pherl , this patch makes protoc break down large Java methods so the generated code of some very large .proto files can now compile (whereas they previously fail with "code too large" errors"). Does this sound good to you? I'll look at the details of this patch if the general approach looks good to you. |
|
@pherl - any thoughts? |
|
@pherl - ping. Does this general approach seem sound? The only bit I don't like much about this is that splitting up the gigantic |
|
@pherl - ping. Does this general approach seem sound? Do I need to revise it for changes that have happened since I initially made the pull request? I wish to stress that this change addresses a real issue we actually have in the field - currently, we're reduced to taking the generated java code and post-processing it with a python script to get something that can compile. |
|
Sorry for the delay. The general approach looks good to me. Do you have any idea what the threshold is for javac? |
| @@ -62,6 +63,19 @@ namespace java { | |||
|
|
|||
| namespace { | |||
|
|
|||
| struct FieldDescriptorCompare { | |||
| bool operator ()(const FieldDescriptor* f1, const FieldDescriptor* f2) { | |||
| if(f1== NULL) { | |||
|
@pherl - The JVM spec. limits bytecode per method to 64K. (Note that all the static initialization inside a class happens inside a single method) This is why my patch does a new method at 32K - then, even if the bytecode estimates are off by a factor of two, the result will still compile. |
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
|
Yes, both |
|
this good to merge? ping @xfxyjwf |
| @@ -67,8 +67,8 @@ class ExtensionGenerator { | |||
| virtual ~ExtensionGenerator() {} | |||
|
|
|||
| virtual void Generate(io::Printer* printer) = 0; | |||
| virtual void GenerateNonNestedInitializationCode(io::Printer* printer) = 0; | |||
| virtual void GenerateRegistrationCode(io::Printer* printer) = 0; | |||
| virtual int GenerateNonNestedInitializationCode(io::Printer* printer) = 0; | |||
There was a problem hiding this comment.
Please document the return value of these two methods.
|
CLAs look good, thanks! |
|
Thanks. Merging. |
Fix java compilation issues when processing large .proto files
Fix parsing sub-message field.
…va_code Fix java compilation issues when processing large .proto files
Fix parsing sub-message field.
Fix issues 579 and 501 on the code.google.com issues list.
Specifically, large .proto files lead to too much static code, leading to a
compilation error from javac: "code too large". This divides the code used
in static initialization into multiple methods to avoid that error. Also,
this incorporates the fix in issue 501 on the code.google.com issues list
to call registry.add only once per extension.