Skip to content

Commit

Permalink
Optimise CodedOutputStream.ArrayEncoder.writeFixed32NoTag/writeFixed6…
Browse files Browse the repository at this point in the history
…4NoTag

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
  • Loading branch information
mhansen authored and copybara-github committed Sep 25, 2024
1 parent b404010 commit a51f98c
Showing 1 changed file with 18 additions and 16 deletions.
34 changes: 18 additions & 16 deletions java/core/src/main/java/com/google/protobuf/CodedOutputStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit a51f98c

Please sign in to comment.