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

Protobuf java generate to large methods #10247

Open
Okapist opened this issue Jul 12, 2022 · 1 comment
Open

Protobuf java generate to large methods #10247

Okapist opened this issue Jul 12, 2022 · 1 comment

Comments

@Okapist
Copy link
Contributor

Okapist commented Jul 12, 2022

What version of protobuf and what language are you using?
Version: main
Language: Java

What operating system (Linux, Windows, ...) and version?
Any

What runtime / compiler are you using (e.g., python version or gcc version)
Any

What did you do?
Steps to reproduce the behavior:

Create .proto file with around 1.000 fields like

message LargeMessage {
optional string s2 = 2;
optional int32 i3 = 3;
....
optional string s998 = 998;
optional int32 i999 = 99;
}

Compile it with protoc

You get too large methods for Java JIT compiler. Default compilation limit is 8000 bytes of byte code.
It will work, but in interpreter mode. Very very slow.

What did you expect to see
bytecode_estimate calculation and split large methods with MaybeRestartJavaMethod(...)

What did you see instead?
Large methods

Additional
It may be fixed with some java flags https://stackoverflow.com/questions/57005557/jit-c2-method-maximum-size
But it's not good solution. Split large method it's better.

@fowles
Copy link
Contributor

fowles commented Jul 12, 2022

I doubt we will have time to pursue this optimization any time soon. If you are feeling ambitious, you could try to implement it, but I do not think this would be a small thing.

copybara-service bot pushed a commit that referenced this issue Nov 1, 2022
… of fields.

Changes to make this improvement:
1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax.
2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints)
3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step.

For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247.

PiperOrigin-RevId: 485057663
copybara-service bot pushed a commit that referenced this issue Nov 2, 2022
… of fields.

Changes to make this improvement:
1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax.
2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints)
3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step.

For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247.

PiperOrigin-RevId: 485057663
copybara-service bot pushed a commit that referenced this issue Nov 2, 2022
… of fields.

Changes to make this improvement:
1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax.
2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints)
3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step.

For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247.

PiperOrigin-RevId: 485057663
copybara-service bot pushed a commit that referenced this issue Nov 3, 2022
… of fields.

Changes to make this improvement:
1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax.
2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints)
3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step.

For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247.

PiperOrigin-RevId: 485057663
copybara-service bot pushed a commit that referenced this issue Nov 3, 2022
… of fields.

Changes to make this improvement:
1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax.
2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints)
3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step.

For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247.

PiperOrigin-RevId: 485952448
googleberg pushed a commit that referenced this issue Nov 10, 2022
… of fields.

Changes to make this improvement:
1) All non-repeated builder fields (including maps) now have a presence bit regardless of syntax.
2) The buildPartial method is split into one method per 32-field block (aligned with bit-mask ints)
3) If a presence bit-mask int is set to 0, no fields are present, so we can skip the logic for all of those fields in the buildPartial step.

For messages with a lot of fields (> 100) that are sparsely populated this can result in a significant improvement. Not only does it potentially skip a lot of field copying logic, but also breaks the buildPartial method into chunks that should be more easily digested by the JIT compiler as discussed in this issue: #10247.

PiperOrigin-RevId: 485952448
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

3 participants