Regression: Removal of parsing constructor causes 2–3× Slower Java proto parsing for deeply nested messages

[amishra-u]

What version of protobuf and what language are you using?
Version: main/v3.25 (also verified the issue on the latest version)
Language: Java

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

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

What did you do?
Steps to reproduce the behavior:
Try to parse below message from byte stream, the parsing is ~2.5x slower in version 3.21.7+.

syntax = "proto3";

option java_package = "com.uber.debugprotobuf";
option java_outer_classname = "DebugProtobufProto";

message BenchmarkRequest {
  NodeA root = 1;
}

message BenchmarkResponse {
}

// Tree structure for benchmarking
// Level 1
message NodeA {
  NodeB child1 = 1;
  NodeB child2 = 2;
  NodeB child3 = 3;
  NodeB child4 = 4;
  NodeB child5 = 5;
  NodeB child6 = 6;
  NodeB child7 = 7;
  NodeB child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 2
message NodeB {
  NodeC child1 = 1;
  NodeC child2 = 2;
  NodeC child3 = 3;
  NodeC child4 = 4;
  NodeC child5 = 5;
  NodeC child6 = 6;
  NodeC child7 = 7;
  NodeC child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 3
message NodeC {NodeD child1 = 1;
  NodeD child2 = 2;
  NodeD child3 = 3;
  NodeD child4 = 4;
  NodeD child5 = 5;
  NodeD child6 = 6;
  NodeD child7 = 7;
  NodeD child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 4
message NodeD {
  NodeE child1 = 1;
  NodeE child2 = 2;
  NodeE child3 = 3;
  NodeE child4 = 4;
  NodeE child5 = 5;
  NodeE child6 = 6;
  NodeE child7 = 7;
  NodeE child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 5
message NodeE {
  NodeF child1 = 1;
  NodeF child2 = 2;
  NodeF child3 = 3;
  NodeF child4 = 4;
  NodeF child5 = 5;
  NodeF child6 = 6;
  NodeF child7 = 7;
  NodeF child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 6
message NodeF {
  NodeG child1 = 1;
  NodeG child2 = 2;
  NodeG child3 = 3;
  NodeG child4 = 4;
  NodeG child5 = 5;
  NodeG child6 = 6;
  NodeG child7 = 7;
  NodeG child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 7
message NodeG {
  NodeH child1 = 1;
  NodeH child2 = 2;
  NodeH child3 = 3;
  NodeH child4 = 4;
  NodeH child5 = 5;
  NodeH child6 = 6;
  NodeH child7 = 7;
  NodeH child8 = 8;
  string name = 9;
  int64 value = 10;
}

// Level 8
message NodeH {
  string name = 1;
  int64 value = 2;
}

Issue Summary
A performance regression was introduced by PR #10665
, where the Java code generator removed the optimized parsing constructor and moved all parsing logic to the Builder. This change results in a 2–3× performance degradation with builder approach.

Before this PR, the Java generator emitted a specialized private parsing constructor for each message type whenever the file was not configured with optimize_for = CODE_SIZE.

• This constructor provided a fast path for parsing: it directly populated message fields while reading from the input stream.
• The builder path was only used for users explicitly optimizing for file size, accepting lower speed.

In PR #10665, this constructor was removed entirely. The changelog even states:

Move proto wireformat parsing functionality from the private “parsing constructor” to the Builder class.

As a result, all parsing now goes through the Builder, regardless of optimization settings.

Fix
Reintroduced the parsing constructor and confirmed there is no performance degradation.
https://github.com/protocolbuffers/protobuf/compare/v25.5...amishra-u:protobuf:v3.25.5-UBER1?expand=1

[honglooker]

@mkruskal-google this hearkens to #10665. Should we consider reverting or close as WAI?

[esrauchg]

Can you clarify how you arrived that it is more performant to readd it?

The cited change that removed it claimed "This is more performant with less code.", do you believe it measured something different than you or that the behavior is envirnoment-specific?

[amishra-u]

I believe the performance improvement being mentioned refers to enhancements in the merging logic that are part of the builder-based approach (& in some case constructor path). The parsing constructor, however, consistently delivers better performance, which I confirmed by running benchmark on the proto in pr summary.

[esrauchg]

I can try to find time to investigate in the new year, but benchmarking JavaProto correctly is extremely difficult due to the JIT being so finicky. I've been looking at it recently and it is common that JMH benchmarks can show a 3x speedup when it is a 3x slowdown in real production contexts.

It's not really plausible that we regressed JavaProto parse path on deeply nested message by 3x in general: most large Google products use JavaProto and have tooling and regression detection and they mostly have large deep message structures. We get notices from the product owners them even on latency neutral changes if it e.g. increases Eden GC. Empirically at least for the contexts that Google runs software in, this change would have been reverted almost immediately if it has caused a 1.3x slowdown on parsing deeply nested messages.

I would vaguely suspect that it may be something like you are seeing it showing as a 2-3x slowdown on benchmarking interpreted JVM but under a hot JIT JVM parse didn't regress or something similar to that here.

[esrauchg]

Investigated this a bit more and the referenced #10665 is also the fix for this CVE, where the issue was that the parsing constructor path being there can cause adversarial inputs to be arbitrary costly in parsing due to it converting back and forth between the builder and built representations many times.

So very straightforward, we could not take the patch to revert it directly, as it would reintroduce the issue as identified in that CVE. But, if there actually was a 2x slowdown from that fix compared to tip of main we would still jump at the clear opportunity to capture a 2x speedup compared to today some other way that wouldn't reintroduce the CVE. Though as I mentioned, it's not plausible that Google's own servers observed a 2x slowdown in this case, and just looking at the patch delta I do suspect that it is exatly shape of thing that would sensibly be a 2x slowdown on interpreted code but not warm-JIT code (transient objects and method dispatches that should evaporate off in reality).

I took a look at the revert proposal at https://github.com/protocolbuffers/protobuf/compare/v25.5...amishra-u:protobuf:v3.25.5-UBER1?expand=1 and it was based 25.x branch; 25 branch is currently on LTS support now which means critical bug fixes only.

A large number of subtle changes including performance impacts have gone in since then time; I'd recommend you evaluate against 4.33 and if you have still find that against 4.33 you can reproduce what looks to be a significant speedup, and how you measured that, then I'm happy to take another look.

(You may have been targeting the 25 branch because 26 was officially a breaking change release for JavaProto, but based on feedback from customers the majority of our breaking changes have actually been rolled back as of 33 release, so we don't expect that anyone should be stuck on the 25 release as of now anymore.)

Thanks!

[amishra-u]

Re: Since no Google services reported issues, is it possible that proto parsing does not account for a significant portion of API latency? It may also be the case that none of the services spend a signification amount of CPU time in proto parsing.

At Uber, the issue was clearly visible in only 2-3 services, which were spending a significant amount of CPU time (~10-20%) in proto parsing (combined avg for all services is 0.7%).

Also verified this against version 4.29 and performance is still roughly ~1.5× slower compared to when the parsing constructor was available.

Based on this, it appears that the primary reason for removing the parsing constructor was the CVE.

[esrauchg]

, is it possible that proto parsing does not account for a significant portion of API latency? It may also be the case that none of the services spend a signification amount of CPU time in proto parsing.

This wouldn't explain it; several of Google's biggest products have expensive JavaProto parses on the critical path of user serving and regression analysis that runs over every individual PR that should catch +20% regressions on parse latency, much less +100%.

I think there's many other ways that both can be true though; JVM tuning settings including tiered compilation on or off can make changes of this shape good or bad, or maybe the topic is sensitive to the shape of the schema or data (for example, Proto2 and now Edition 2024 are what are generally used inside Google, compared to Proto3 being dominant outside of Google, which has a lot of subtle implications). There also are differences between Protobuf runtime as run internally and the OSS releases, but I don't immediately have any reason to suspect those differences should be relevant in this case.

If you can rebase a proposal that you think is a big speedup to tip of main I can try to evaluate it here. If we 'know' that we couldn't take it verbatim because of the CVE risk but it proves there's a 2x speedup available on 'sane' shapes of schema/data then there's absolutely something that we can and will move on, since we do spend a lot of time for optimization much smaller than that.

[amishra-u]

@esrauchg I’m seeing a strong signal that removing the parsing constructor causes a performance regression. I’ve added benchmarks to cover both:

1. The regression caused by removing the parsing constructor
2. CVE-2022-3171

The results show that removal of the parsing constructor leads to ~2× higher parsing latency on the current head. It also confirm that my patch reintroduces the CVE.

The parsing-latency benchmarks were run against C2-compiled code (fully optimized). If there are concerns, we can enable compilation logs and correlate the benchmark start time with the final C2 compilation event.

Current head

Steps

1. Check out this commit
2. Run bazel run //java/benchmark:bin_benchmark

Results

• Mean: ~189 ms/op
• P95: ~208 ms/op

With patch (parsing constructor reintroduced)

Steps

1. Check out this commit
2. Run bazel run //java/benchmark:bin_benchmark

Results

• Mean: ~78 ms/op
• P95: ~81 ms/op

This represents ~2× improvement when the parsing constructor is present.

CVE confirmation

Current head

Steps

1. Check out this commit
2. Run bazel run //java/benchmark:bin_cve --jvmopt=-verbose:gc

Result

No GC pauses observed.

With patch

Steps

1. Check out this commit
2. Run bazel run //java/benchmark:bin_cve --jvmopt=-verbose:gc

Result

Parsing single malicious payload triggers 70+ GC pauses.