From f7b0faf55837c1106c6dc16985058b2c0747ae92 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Tue, 8 Oct 2024 07:42:04 -0700 Subject: [PATCH] Speed up SingleFieldBuilder.getBuilder() by avoiding reloading this.builder at return. See godbolt for Android ART compiler: https://godbolt.org/z/M9dWhdqbf This optimisation brings the implementation down from 284 bytes to 272 bytes. - `GeneratedMessage$Builder SingleFieldBuilder.getBuilder() [284 bytes]` - `GeneratedMessage$Builder SingleFieldBuilder.getBuilder__withLocalVariable() [272 bytes]` It's not big. It's just a few instructions dropped. These were dropped just before the `ret`: ``` -mov x23, x1 -ldr w0, [x23, #8] ``` And this load dropped before calling `markClean`. ``` -ldr w1, [x23, #8] ``` PiperOrigin-RevId: 683619150 --- .../main/java/com/google/protobuf/SingleFieldBuilder.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/java/core/src/main/java/com/google/protobuf/SingleFieldBuilder.java b/java/core/src/main/java/com/google/protobuf/SingleFieldBuilder.java index 17326cb0b8782..9697b6ea30b9a 100644 --- a/java/core/src/main/java/com/google/protobuf/SingleFieldBuilder.java +++ b/java/core/src/main/java/com/google/protobuf/SingleFieldBuilder.java @@ -100,12 +100,18 @@ public MType build() { */ @SuppressWarnings("unchecked") public BType getBuilder() { + // This code is very hot. + // Optimisation: store this.builder in a local variable so that the compiler doesn't reload + // it at 'return'. Android's compiler thinks the methods called between assignment & return + // might reassign this.builder so the compiler can't make this optimisation without our help. + BType builder = this.builder; + if (builder == null) { // builder.mergeFrom() on a fresh builder // does not create any sub-objects with independent clean/dirty states, // therefore setting the builder itself to clean without actually calling // build() cannot break any invariants. - builder = (BType) message.newBuilderForType(this); + this.builder = builder = (BType) message.newBuilderForType(this); builder.mergeFrom(message); // no-op if message is the default message builder.markClean(); }