From a51f98ce0c211492d070970f214a6eefb6aadf63 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Tue, 24 Sep 2024 23:01:10 -0700 Subject: [PATCH] Optimise CodedOutputStream.ArrayEncoder.writeFixed32NoTag/writeFixed64NoTag On Android, this generates better assembly code, bounds-checking through all the used indices upfront, and branching to deoptimise if it's not true, avoiding doing 4x bounds checks. We also don't generate 4 different `pThrowArrayBounds` code sections. https://godbolt.org/z/Kbhvcdvbd Code size Comparison: - `void X.writeFixed32NoTag__before(int) [292 bytes]` - `void X.writeFixed32NoTag__after(int) [180 bytes]` This starts by throwing a more meaningful length (4bytes or 8bytes for fixed64), which makes sure the value of position in the catch clause isn't dependent on which line threw the exception. PiperOrigin-RevId: 678543462 --- .../google/protobuf/CodedOutputStream.java | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java b/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java index 532ff3ad99b8..57be310f0027 100644 --- a/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java +++ b/java/core/src/main/java/com/google/protobuf/CodedOutputStream.java @@ -1348,15 +1348,16 @@ public final void writeUInt32NoTag(int value) throws IOException { public final void writeFixed32NoTag(int value) throws IOException { int position = this.position; // Perf: hoist field to register to avoid load/stores. try { - buffer[position++] = (byte) (value & 0xFF); - buffer[position++] = (byte) ((value >> 8) & 0xFF); - buffer[position++] = (byte) ((value >> 16) & 0xFF); - buffer[position++] = (byte) ((value >> 24) & 0xFF); + buffer[position] = (byte) (value & 0xFF); + buffer[position + 1] = (byte) ((value >> 8) & 0xFF); + buffer[position + 2] = (byte) ((value >> 16) & 0xFF); + buffer[position + 3] = (byte) ((value >> 24) & 0xFF); } catch (IndexOutOfBoundsException e) { throw new OutOfSpaceException( - String.format("Pos: %d, limit: %d, len: %d", position, limit, 1), e); + String.format("Pos: %d, limit: %d, len: %d", position, limit, FIXED32_SIZE), e); } - this.position = position; // Only update position if we stayed within the array bounds. + // Only update position if we stayed within the array bounds. + this.position = position + FIXED32_SIZE; } @Override @@ -1393,19 +1394,20 @@ public final void writeUInt64NoTag(long value) throws IOException { public final void writeFixed64NoTag(long value) throws IOException { int position = this.position; // Perf: hoist field to register to avoid load/stores. try { - buffer[position++] = (byte) ((int) (value) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 8) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 16) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 24) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 32) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 40) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 48) & 0xFF); - buffer[position++] = (byte) ((int) (value >> 56) & 0xFF); + buffer[position] = (byte) ((int) (value) & 0xFF); + buffer[position + 1] = (byte) ((int) (value >> 8) & 0xFF); + buffer[position + 2] = (byte) ((int) (value >> 16) & 0xFF); + buffer[position + 3] = (byte) ((int) (value >> 24) & 0xFF); + buffer[position + 4] = (byte) ((int) (value >> 32) & 0xFF); + buffer[position + 5] = (byte) ((int) (value >> 40) & 0xFF); + buffer[position + 6] = (byte) ((int) (value >> 48) & 0xFF); + buffer[position + 7] = (byte) ((int) (value >> 56) & 0xFF); } catch (IndexOutOfBoundsException e) { throw new OutOfSpaceException( - String.format("Pos: %d, limit: %d, len: %d", position, limit, 1), e); + String.format("Pos: %d, limit: %d, len: %d", position, limit, FIXED64_SIZE), e); } - this.position = position; // Only update position if we stayed within the array bounds. + // Only update position if we stayed within the array bounds. + this.position = position + FIXED64_SIZE; } @Override