From 873108ec965024ad686da31901b6f121b9a4f6e7 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 30 Jul 2026 08:30:47 +0200 Subject: [PATCH 1/6] =?UTF-8?q?WIP:=20unchecked=20Netty=20reads=20via=20io?= =?UTF-8?q?.netty.buffer=20helper=20=E2=80=94=20parked,=20do=20not=20merge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured and rejected (2026-07-30): a generated helper class in the io.netty.buffer package reaches AbstractByteBuf's protected _getByte / _getIntLE / _getLongLE and the readerIndex field directly, decoding a whole varint with zero per-byte checks and a single index store. The codec gates each read on `instanceof AbstractByteBuf && readable >= 10` and falls back to the checked chain near the buffer end. Despite removing every check, ALL deserialize benchmarks regressed (same-session baseline, 2 forks, tight bars): Simple 87.0 -> 59.1 (-32%) Simple+readString 51.3 -> 30.7 (-40%) BaseCommand 46.8 -> 32.3 (-31%) AddressBook 27.3 -> 20.2 (-26%) MessageMetadata 17.4 -> 15.9 (-8%) Interpretation: C2 optimizes the canonical checkReadableBytes0()/ readByte() pattern to near-free (the Netty kill-switch flags showed the checks cost only 8-17% when removed globally with no gate); the two-path gate plus a hand-built chain defeats that. Preserved for reference only — see the perf-plan gist, section 4. --- .../generator/LightProtoGenerator.java | 25 ++- .../generator/LightProtoByteBufAccess.java | 155 ++++++++++++++++++ .../lightproto/generator/LightProtoCodec.java | 28 ++++ 3 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java diff --git a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java index c8f4a2a..a1201e3 100644 --- a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java +++ b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java @@ -89,6 +89,29 @@ public static List generate(List descriptors, File ou // Include the coded class once per every generated java package for (String javaPackage : javaPackages) { + // Unchecked ByteBuf accessor companion: lives in the io.netty.buffer + // package (to reach AbstractByteBuf's protected members) and is named + // after the generated package so that multiple generated packages in + // one project don't collide. + String accessClassName = "LightProtoByteBufAccess_" + javaPackage.replace('.', '_'); + try (InputStream is = LightProtoGenerator.class.getResourceAsStream( + "/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java")) { + String accessSource = new String(is.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8) + .replace("LightProtoByteBufAccessTemplate", accessClassName); + Path nettyDir = Paths.get(String.format("%s/io/netty/buffer", outputDirectory)); + Files.createDirectories(nettyDir); + Path accessFile = nettyDir.resolve(accessClassName + ".java"); + try (Writer w = Files.newBufferedWriter(accessFile)) { + w.write(String.format( + "/*%n" + + " * Generated by LightProto v%s%n" + + " * DO NOT MODIFY%n" + + " * DO NOT CHECK IN TO SOURCE CONTROL%n" + + " */%n", VERSION)); + w.write(accessSource); + } + } + try (InputStream is = LightProtoGenerator.class.getResourceAsStream("/io/streamnative/lightproto/generator/LightProtoCodec.java")) { JavaClassSource codecClass = (JavaClassSource) Roaster.parse(is); codecClass.setPackage(javaPackage); @@ -102,7 +125,7 @@ public static List generate(List descriptors, File ou " * DO NOT MODIFY%n" + " * DO NOT CHECK IN TO SOURCE CONTROL%n" + " */%n", VERSION)); - w.write(codecClass.toString()); + w.write(codecClass.toString().replace("LightProtoByteBufAccessTemplate", accessClassName)); } } } diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java new file mode 100644 index 0000000..5660a39 --- /dev/null +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java @@ -0,0 +1,155 @@ +/** + * Copyright 2026 StreamNative + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.netty.buffer; + +/** + * Placed in the {@code io.netty.buffer} package to reach {@link AbstractByteBuf}'s + * protected unchecked accessors ({@code _getByte} etc.) and its {@code readerIndex} + * field directly. Callers validate readability once for the whole access, so the + * per-byte {@code checkReadableBytes0()} / {@code ensureAccessible()} checks that + * dominate parse profiles are skipped, and each varint performs a single + * {@code readerIndex} store instead of one per byte. + * + *

NOTE: this class creates a split package with netty-buffer under the JPMS + * module path; it is intended for classpath deployments. + */ +public final class LightProtoByteBufAccessTemplate { + + private LightProtoByteBufAccessTemplate() { + } + + /** Readable bytes via direct field access (no accessibility check). */ + public static int readableBytesFast(AbstractByteBuf b) { + return b.writerIndex - b.readerIndex; + } + + /** + * Read a varint of up to 10 bytes, discarding bits above 32. + * The caller must have verified at least 10 readable bytes. + */ + public static int readVarIntUnchecked(AbstractByteBuf buf) { + int i = buf.readerIndex; + byte tmp = buf._getByte(i++); + if (tmp >= 0) { + buf.readerIndex = i; + return tmp; + } + int result = tmp & 0x7f; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= tmp << 7; + } else { + result |= (tmp & 0x7f) << 7; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= tmp << 14; + } else { + result |= (tmp & 0x7f) << 14; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= tmp << 21; + } else { + result |= (tmp & 0x7f) << 21; + result |= (tmp = buf._getByte(i++)) << 28; + if (tmp < 0) { + // Discard upper 32 bits. + for (int j = 0; j < 5; j++) { + if (buf._getByte(i++) >= 0) { + buf.readerIndex = i; + return result; + } + } + buf.readerIndex = i; + throw new IllegalArgumentException("Encountered a malformed varint."); + } + } + } + } + buf.readerIndex = i; + return result; + } + + /** + * Read a varint of up to 10 bytes as a long. + * The caller must have verified at least 10 readable bytes. + */ + public static long readVarInt64Unchecked(AbstractByteBuf buf) { + int i = buf.readerIndex; + long result; + byte tmp = buf._getByte(i++); + if (tmp >= 0) { + buf.readerIndex = i; + return tmp; + } + result = tmp & 0x7fL; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 7; + } else { + result |= (tmp & 0x7fL) << 7; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 14; + } else { + result |= (tmp & 0x7fL) << 14; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 21; + } else { + result |= (tmp & 0x7fL) << 21; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 28; + } else { + result |= (tmp & 0x7fL) << 28; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 35; + } else { + result |= (tmp & 0x7fL) << 35; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 42; + } else { + result |= (tmp & 0x7fL) << 42; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 49; + } else { + result |= (tmp & 0x7fL) << 49; + if ((tmp = buf._getByte(i++)) >= 0) { + result |= (long) tmp << 56; + } else { + result |= (tmp & 0x7fL) << 56; + result |= ((long) buf._getByte(i++)) << 63; + } + } + } + } + } + } + } + } + buf.readerIndex = i; + return result; + } + + /** Read a little-endian fixed 32-bit value; caller verified 4 readable bytes. */ + public static int readFixedInt32Unchecked(AbstractByteBuf buf) { + int i = buf.readerIndex; + int v = buf._getIntLE(i); + buf.readerIndex = i + 4; + return v; + } + + /** Read a little-endian fixed 64-bit value; caller verified 8 readable bytes. */ + public static long readFixedInt64Unchecked(AbstractByteBuf buf) { + int i = buf.readerIndex; + long v = buf._getLongLE(i); + buf.readerIndex = i + 8; + return v; + } +} diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java index f2840a0..2f6a438 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java @@ -15,6 +15,7 @@ */ package io.streamnative.lightproto.generator; +import io.netty.buffer.AbstractByteBuf; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufUtil; @@ -188,10 +189,22 @@ static void writeFixedInt64(ByteBuf b, long n) { } static int readFixedInt32(ByteBuf b) { + if (b instanceof AbstractByteBuf) { + AbstractByteBuf a = (AbstractByteBuf) b; + if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 4) { + return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt32Unchecked(a); + } + } return b.readIntLE(); } static long readFixedInt64(ByteBuf b) { + if (b instanceof AbstractByteBuf) { + AbstractByteBuf a = (AbstractByteBuf) b; + if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 8) { + return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt64Unchecked(a); + } + } return b.readLongLE(); } @@ -217,6 +230,15 @@ private static long decodeZigZag64(long n) { } static int readVarInt(ByteBuf buf) { + // With >= 10 readable bytes a varint cannot overrun the buffer, so the + // per-byte readByte() checks (accessibility + bounds + an index store per + // byte) can be skipped entirely via the unchecked accessors. + if (buf instanceof AbstractByteBuf) { + AbstractByteBuf a = (AbstractByteBuf) buf; + if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 10) { + return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarIntUnchecked(a); + } + } byte tmp = buf.readByte(); if (tmp >= 0) { return tmp; @@ -251,6 +273,12 @@ static int readVarInt(ByteBuf buf) { } static long readVarInt64(ByteBuf buf) { + if (buf instanceof AbstractByteBuf) { + AbstractByteBuf a = (AbstractByteBuf) buf; + if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 10) { + return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarInt64Unchecked(a); + } + } long result; byte tmp = buf.readByte(); if (tmp >= 0) { From 05ec56813a32a9234cfe859b4969231a4eb9b1b1 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 30 Jul 2026 08:53:15 +0200 Subject: [PATCH 2/6] Drop the per-varint readable gates; safety moves to message level Removing the `readableBytesFast(a) >= N` gate per read flipped the experiment from a uniform regression to a real win: the gate branch was the entire cost of the previous commit, not the unchecked chain. Safety is now enforced once per message instead of once per varint: - A truncated field always consumes past the message limit, so a single post-parse `readerIndex > endIdx` check restores the throw-on-truncated contract (covered by TruncatedInputTest across heap and pooled-direct buffers). - Overruns are bounded at 9 bytes. Heap buffers fail safely (ArrayIndexOutOfBoundsException is an IndexOutOfBoundsException) and pooled direct buffers read within their arena chunk. The residual exposure is a truncated trailing varint on a tight-capacity unpooled unsafe-direct buffer. Deserialize throughput vs the checked baseline (JMH, Apple M-series, 2-4 forks; later runs on this machine showed degraded/noisy conditions, so a long-run confirmation matrix is required before merging): MessageMetadata 17.4 -> 23.7 (+36%, reproducible across runs) Simple 87.0 -> 99.4 (+14%) AddressBook 27.3 -> 27.5 (flat) BaseCommand 46.8 -> 43.4 (-7%, within fork variance) Simple+readString 51.3 -> 48.2 (-6%) --- .../generator/LightProtoMessage.java | 6 + .../lightproto/generator/LightProtoCodec.java | 26 ++--- .../lightproto/tests/TruncatedInputTest.java | 103 ++++++++++++++++++ 3 files changed, 116 insertions(+), 19 deletions(-) create mode 100644 tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java diff --git a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java index 7367b1e..39c8b94 100644 --- a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java +++ b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java @@ -173,6 +173,12 @@ private void generateParseFrom(PrintWriter w) { w.format(" LightProtoCodec.skipUnknownField(_tag, _buffer);\n"); w.format(" }\n"); w.format(" }\n"); + // A truncated field always consumes past the message limit (the unchecked + // readers don't stop at _endIdx), so one check restores the + // throw-on-truncated-input contract. + w.format(" if (_buffer.readerIndex() > _endIdx) {\n"); + w.format(" throw new IndexOutOfBoundsException(\"Truncated protobuf message\");\n"); + w.format(" }\n"); if (hasRequiredFields()) { w.format(" checkRequiredFields();\n"); } diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java index 2f6a438..ce88707 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java @@ -190,20 +190,14 @@ static void writeFixedInt64(ByteBuf b, long n) { static int readFixedInt32(ByteBuf b) { if (b instanceof AbstractByteBuf) { - AbstractByteBuf a = (AbstractByteBuf) b; - if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 4) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt32Unchecked(a); - } + return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt32Unchecked((AbstractByteBuf) b); } return b.readIntLE(); } static long readFixedInt64(ByteBuf b) { if (b instanceof AbstractByteBuf) { - AbstractByteBuf a = (AbstractByteBuf) b; - if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 8) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt64Unchecked(a); - } + return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt64Unchecked((AbstractByteBuf) b); } return b.readLongLE(); } @@ -230,14 +224,11 @@ private static long decodeZigZag64(long n) { } static int readVarInt(ByteBuf buf) { - // With >= 10 readable bytes a varint cannot overrun the buffer, so the - // per-byte readByte() checks (accessibility + bounds + an index store per - // byte) can be skipped entirely via the unchecked accessors. + // The unchecked chain skips the per-byte readByte() checks (accessibility + // + bounds + an index store per byte); a truncated message can overrun by + // at most 9 bytes, which the generated parseFrom() detects afterwards. if (buf instanceof AbstractByteBuf) { - AbstractByteBuf a = (AbstractByteBuf) buf; - if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 10) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarIntUnchecked(a); - } + return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarIntUnchecked((AbstractByteBuf) buf); } byte tmp = buf.readByte(); if (tmp >= 0) { @@ -274,10 +265,7 @@ static int readVarInt(ByteBuf buf) { static long readVarInt64(ByteBuf buf) { if (buf instanceof AbstractByteBuf) { - AbstractByteBuf a = (AbstractByteBuf) buf; - if (io.netty.buffer.LightProtoByteBufAccessTemplate.readableBytesFast(a) >= 10) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarInt64Unchecked(a); - } + return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarInt64Unchecked((AbstractByteBuf) buf); } long result; byte tmp = buf.readByte(); diff --git a/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java b/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java new file mode 100644 index 0000000..5a620da --- /dev/null +++ b/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java @@ -0,0 +1,103 @@ +/** + * Copyright 2026 StreamNative + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.streamnative.lightproto.tests; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.PooledByteBufAllocator; +import io.netty.buffer.Unpooled; +import org.junit.jupiter.api.Test; + +/** + * Truncated input must throw IndexOutOfBoundsException: heap buffers fail + * safely on array bounds, and for direct buffers the unchecked readers overrun + * by at most 9 bytes before the post-parse limit check detects it. + */ +public class TruncatedInputTest { + + // int_field tag followed by a dangling continuation byte + private static final byte[] TRUNCATED_VARINT = {0x08, (byte) 0x80}; + // long_field (tag 0x10) truncated mid-varint64 + private static final byte[] TRUNCATED_VARINT64 = {0x10, (byte) 0xFF, (byte) 0xFF}; + // double_field (tag 0x21) with only 3 of 8 bytes + private static final byte[] TRUNCATED_FIXED64 = {0x21, 0x00, 0x00, 0x00}; + + private static void assertThrowsOnAllBufferTypes(byte[] data) { + // Heap, exact size + assertThrows(IndexOutOfBoundsException.class, + () -> new Proto3Message().parseFrom(data)); + + // Pooled direct (capacity slack: overrun reads stay in the chunk, the + // post-parse limit check throws) + ByteBuf pooled = PooledByteBufAllocator.DEFAULT.directBuffer(data.length); + try { + pooled.writeBytes(data); + assertThrows(IndexOutOfBoundsException.class, + () -> new Proto3Message().parseFrom(pooled, pooled.readableBytes())); + } finally { + pooled.release(); + } + + } + + @Test + public void testTruncatedVarint() { + assertThrowsOnAllBufferTypes(TRUNCATED_VARINT); + } + + @Test + public void testTruncatedVarint64() { + assertThrowsOnAllBufferTypes(TRUNCATED_VARINT64); + } + + @Test + public void testTruncatedFixed64() { + assertThrowsOnAllBufferTypes(TRUNCATED_FIXED64); + } + + @Test + public void testTruncatedLengthDelimited() { + // string_field (tag 0x32) claims 5 bytes but only 2 are present: + // the checked skipBytes() catches this + byte[] data = {0x32, 0x05, 'a', 'b'}; + assertThrowsOnAllBufferTypes(data); + } + + @Test + public void testValidMessageOnTightCapacityBuffer() { + // A valid message on a zero-slack buffer never overruns + Proto3Message src = new Proto3Message(); + src.setIntField(42); + src.setLongField(123456789L); + src.setStringField("tight-capacity"); + byte[] data = src.toByteArray(); + + ByteBuf tight = Unpooled.directBuffer(data.length, data.length); + try { + tight.writeBytes(data); + Proto3Message parsed = new Proto3Message(); + parsed.parseFrom(tight, tight.readableBytes()); + assertEquals(42, parsed.getIntField()); + assertEquals(123456789L, parsed.getLongField()); + assertEquals("tight-capacity", parsed.getStringField()); + assertEquals(0, tight.readableBytes()); + } finally { + tight.release(); + } + } +} From 930746a7cbb0e76c15f0fed324e6f9358a4df1f8 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 30 Jul 2026 09:25:51 +0200 Subject: [PATCH 3/6] Restrict unchecked reads to 64-bit varints Full 5-fork matrix vs the checked baseline: the unchecked chain is a large win exactly where multi-byte varints dominate (MessageMetadata +31%, reproducible across every run and measurement condition) and appeared to regress 1-byte-dominated shapes. The regression readings are thermally confounded, however: back-to-back matrix arms on a throttling laptop penalize later arms, and the hybrid arm "regressed" readString -17% with a provably byte-identical hot path (no varint64 fields in Frame/Point at all). Decision-grade adjudication of the checked-path shapes needs an interleaved A/B on a cool machine. This commit keeps the unchecked chain only in readVarInt64(), where the win is certain; readVarInt() and the fixed readers stay on the canonical checked path, which C2 compiles to near-free for the 1-byte case. The post-parse limit check and TruncatedInputTest cover the (at most 9-byte) overrun of a truncated varint64. --- .../lightproto/generator/LightProtoCodec.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java index ce88707..1a2fe33 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java @@ -189,16 +189,10 @@ static void writeFixedInt64(ByteBuf b, long n) { } static int readFixedInt32(ByteBuf b) { - if (b instanceof AbstractByteBuf) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt32Unchecked((AbstractByteBuf) b); - } return b.readIntLE(); } static long readFixedInt64(ByteBuf b) { - if (b instanceof AbstractByteBuf) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readFixedInt64Unchecked((AbstractByteBuf) b); - } return b.readLongLE(); } @@ -224,12 +218,6 @@ private static long decodeZigZag64(long n) { } static int readVarInt(ByteBuf buf) { - // The unchecked chain skips the per-byte readByte() checks (accessibility - // + bounds + an index store per byte); a truncated message can overrun by - // at most 9 bytes, which the generated parseFrom() detects afterwards. - if (buf instanceof AbstractByteBuf) { - return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarIntUnchecked((AbstractByteBuf) buf); - } byte tmp = buf.readByte(); if (tmp >= 0) { return tmp; @@ -264,6 +252,11 @@ static int readVarInt(ByteBuf buf) { } static long readVarInt64(ByteBuf buf) { + // Only 64-bit varints use the unchecked chain: their multi-byte decode + // amortizes it (+31% on varint64-heavy messages), while 1-byte-dominated + // reads measured faster on the canonical checked readByte() path. A + // truncated varint64 can overrun the message limit by at most 9 bytes; + // the generated parseFrom() detects that afterwards. if (buf instanceof AbstractByteBuf) { return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarInt64Unchecked((AbstractByteBuf) buf); } From f1d5c5f7c722d3838661f7cf12ae0f23cad0b816 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 30 Jul 2026 13:26:49 +0200 Subject: [PATCH 4/6] Inline short varint64 reads; emit the truncation check only where needed Interleaved A/B against master showed the varint64-only unchecked path regressing every benchmark without long varints: PrintInlining revealed readVarInt64Unchecked (371 bytecodes) and the readVarInt64 wrapper with the checked chain inline (303 bytecodes) both fail to inline, so 1-2 byte varints paid a call per read; and the unconditional post-parse truncation check perturbed codegen of small messages that cannot overrun at all. - Decode 1-2 byte varint64s in a small always-inlined front, falling into the (still non-inlined) nested chain only for 3+ byte values. - Split the checked fallback out of readVarInt64 so the instanceof gate always inlines. - Emit the post-parse truncation check only for messages that parse 64-bit varints; all other messages keep parseFrom byte-identical to the fully checked version. --- .../lightproto/generator/LightProtoField.java | 13 ++++++++++++ .../generator/LightProtoMapField.java | 5 +++++ .../generator/LightProtoMessage.java | 17 +++++++++------ .../generator/LightProtoByteBufAccess.java | 21 +++++++++++++++++++ .../lightproto/generator/LightProtoCodec.java | 14 +++++++++---- 5 files changed, 60 insertions(+), 10 deletions(-) diff --git a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoField.java b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoField.java index 4417a08..5f0e843 100644 --- a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoField.java +++ b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoField.java @@ -78,6 +78,19 @@ public boolean isRequired() { return field.isRequired(); } + /** + * True when parsing this field invokes LightProtoCodec.readVarInt64(), the one + * reader that can advance past the message limit on truncated input. Messages + * with no such field skip the post-parse truncation check. + */ + public boolean usesVarInt64Parse() { + return isVarInt64Type(field.getProtoType()); + } + + static boolean isVarInt64Type(String protoType) { + return "int64".equals(protoType) || "uint64".equals(protoType) || "sint64".equals(protoType); + } + public void docs(PrintWriter w) { String doc = field.getDoc(); if (doc != null && !doc.isEmpty()) { diff --git a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMapField.java b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMapField.java index e0ea51b..95da500 100644 --- a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMapField.java +++ b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMapField.java @@ -30,6 +30,11 @@ public LightProtoMapField(ProtoFieldDescriptor field, int index) { this.valueField = field.getMapValueField(); } + @Override + public boolean usesVarInt64Parse() { + return isVarInt64Type(keyField.getProtoType()) || isVarInt64Type(valueField.getProtoType()); + } + private boolean isStringKey() { return keyField.isStringField(); } diff --git a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java index 39c8b94..1829821 100644 --- a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java +++ b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMessage.java @@ -173,12 +173,17 @@ private void generateParseFrom(PrintWriter w) { w.format(" LightProtoCodec.skipUnknownField(_tag, _buffer);\n"); w.format(" }\n"); w.format(" }\n"); - // A truncated field always consumes past the message limit (the unchecked - // readers don't stop at _endIdx), so one check restores the - // throw-on-truncated-input contract. - w.format(" if (_buffer.readerIndex() > _endIdx) {\n"); - w.format(" throw new IndexOutOfBoundsException(\"Truncated protobuf message\");\n"); - w.format(" }\n"); + // A truncated varint64 is the one read that can consume past the message + // limit without throwing (every other reader is bounds-checked), so the + // check that restores the throw-on-truncated-input contract is emitted + // only for messages that parse 64-bit varints. Messages without them keep + // parseFrom() byte-identical to the fully checked version: even a dead + // extra check measurably perturbs code generation on small messages. + if (fields.stream().anyMatch(LightProtoField::usesVarInt64Parse)) { + w.format(" if (_buffer.readerIndex() > _endIdx) {\n"); + w.format(" throw new IndexOutOfBoundsException(\"Truncated protobuf message\");\n"); + w.format(" }\n"); + } if (hasRequiredFields()) { w.format(" checkRequiredFields();\n"); } diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java index 5660a39..6c10bd6 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java @@ -82,8 +82,29 @@ public static int readVarIntUnchecked(AbstractByteBuf buf) { /** * Read a varint of up to 10 bytes as a long. * The caller must have verified at least 10 readable bytes. + * + *

The 1-2 byte cases are decoded here so the method stays far below C2's + * FreqInlineSize: the full nested chain compiles to ~371 bytecodes and is + * never inlined, so routing short varints through it costs a call per read — + * a net loss on small values. Longer varints fall into the chain, whose + * multi-byte decode amortizes the call. */ public static long readVarInt64Unchecked(AbstractByteBuf buf) { + int i = buf.readerIndex; + byte b0 = buf._getByte(i); + if (b0 >= 0) { + buf.readerIndex = i + 1; + return b0; + } + byte b1 = buf._getByte(i + 1); + if (b1 >= 0) { + buf.readerIndex = i + 2; + return (b0 & 0x7f) | ((long) b1 << 7); + } + return readVarInt64UncheckedChain(buf); + } + + private static long readVarInt64UncheckedChain(AbstractByteBuf buf) { int i = buf.readerIndex; long result; byte tmp = buf._getByte(i++); diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java index 1a2fe33..62e01ad 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoCodec.java @@ -252,14 +252,20 @@ static int readVarInt(ByteBuf buf) { } static long readVarInt64(ByteBuf buf) { - // Only 64-bit varints use the unchecked chain: their multi-byte decode - // amortizes it (+31% on varint64-heavy messages), while 1-byte-dominated - // reads measured faster on the canonical checked readByte() path. A + // Only 64-bit varints use the unchecked path: it skips the per-byte + // bounds/accessibility checks (+31% on varint64-heavy messages). A // truncated varint64 can overrun the message limit by at most 9 bytes; - // the generated parseFrom() detects that afterwards. + // the generated parseFrom() detects that afterwards. The checked + // fallback lives in its own method so this one stays small enough to + // always inline (with the chain inline it is 303 bytecodes and C2 + // refuses it at hot call sites). if (buf instanceof AbstractByteBuf) { return io.netty.buffer.LightProtoByteBufAccessTemplate.readVarInt64Unchecked((AbstractByteBuf) buf); } + return readVarInt64Checked(buf); + } + + private static long readVarInt64Checked(ByteBuf buf) { long result; byte tmp = buf.readByte(); if (tmp >= 0) { From 63411aba4ddb4acec4fe79ddce16f7ac865cbd0b Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 30 Jul 2026 14:22:17 +0200 Subject: [PATCH 5/6] Trim the buffer-access helper to the varint64 readers actually used readableBytesFast, readVarIntUnchecked and the fixed-int readers were residue of the earlier gated/gateless experiments; nothing references them since unchecked reads were restricted to 64-bit varints. Also update the class javadoc: safety is enforced by the generated parseFrom(), not by caller-side readability validation. --- .../generator/LightProtoByteBufAccess.java | 78 ++----------------- 1 file changed, 8 insertions(+), 70 deletions(-) diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java index 6c10bd6..420a755 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java @@ -17,11 +17,14 @@ /** * Placed in the {@code io.netty.buffer} package to reach {@link AbstractByteBuf}'s - * protected unchecked accessors ({@code _getByte} etc.) and its {@code readerIndex} - * field directly. Callers validate readability once for the whole access, so the - * per-byte {@code checkReadableBytes0()} / {@code ensureAccessible()} checks that - * dominate parse profiles are skipped, and each varint performs a single - * {@code readerIndex} store instead of one per byte. + * protected unchecked accessors ({@code _getByte}) and its {@code readerIndex} + * field directly. The per-byte {@code checkReadableBytes0()} / + * {@code ensureAccessible()} checks that dominate parse profiles are skipped, and + * each varint performs a single {@code readerIndex} store instead of one per byte. + * + *

Reads are not bounds-checked here: on truncated input a read may advance up + * to 10 bytes past the intended message limit. The generated {@code parseFrom()} + * detects that afterwards and throws, preserving the throw-on-truncated contract. * *

NOTE: this class creates a split package with netty-buffer under the JPMS * module path; it is intended for classpath deployments. @@ -31,57 +34,8 @@ public final class LightProtoByteBufAccessTemplate { private LightProtoByteBufAccessTemplate() { } - /** Readable bytes via direct field access (no accessibility check). */ - public static int readableBytesFast(AbstractByteBuf b) { - return b.writerIndex - b.readerIndex; - } - - /** - * Read a varint of up to 10 bytes, discarding bits above 32. - * The caller must have verified at least 10 readable bytes. - */ - public static int readVarIntUnchecked(AbstractByteBuf buf) { - int i = buf.readerIndex; - byte tmp = buf._getByte(i++); - if (tmp >= 0) { - buf.readerIndex = i; - return tmp; - } - int result = tmp & 0x7f; - if ((tmp = buf._getByte(i++)) >= 0) { - result |= tmp << 7; - } else { - result |= (tmp & 0x7f) << 7; - if ((tmp = buf._getByte(i++)) >= 0) { - result |= tmp << 14; - } else { - result |= (tmp & 0x7f) << 14; - if ((tmp = buf._getByte(i++)) >= 0) { - result |= tmp << 21; - } else { - result |= (tmp & 0x7f) << 21; - result |= (tmp = buf._getByte(i++)) << 28; - if (tmp < 0) { - // Discard upper 32 bits. - for (int j = 0; j < 5; j++) { - if (buf._getByte(i++) >= 0) { - buf.readerIndex = i; - return result; - } - } - buf.readerIndex = i; - throw new IllegalArgumentException("Encountered a malformed varint."); - } - } - } - } - buf.readerIndex = i; - return result; - } - /** * Read a varint of up to 10 bytes as a long. - * The caller must have verified at least 10 readable bytes. * *

The 1-2 byte cases are decoded here so the method stays far below C2's * FreqInlineSize: the full nested chain compiles to ~371 bytecodes and is @@ -157,20 +111,4 @@ private static long readVarInt64UncheckedChain(AbstractByteBuf buf) { buf.readerIndex = i; return result; } - - /** Read a little-endian fixed 32-bit value; caller verified 4 readable bytes. */ - public static int readFixedInt32Unchecked(AbstractByteBuf buf) { - int i = buf.readerIndex; - int v = buf._getIntLE(i); - buf.readerIndex = i + 4; - return v; - } - - /** Read a little-endian fixed 64-bit value; caller verified 8 readable bytes. */ - public static long readFixedInt64Unchecked(AbstractByteBuf buf) { - int i = buf.readerIndex; - long v = buf._getLongLE(i); - buf.readerIndex = i + 8; - return v; - } } From ea823cffe5e9642e9b8bfb3c90060c4fb2c2342b Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Thu, 30 Jul 2026 17:46:24 +0200 Subject: [PATCH 6/6] Address review: map truncation coverage, doc bounds, fail-fast template load - Add MapInt64Message and a truncation test proving the post-parse check is emitted when only map key/value types route through readVarInt64(). - Correct the helper javadoc overrun bound: at most 9 bytes past the message limit, not 10. - Narrow TruncatedInputTest's javadoc to the buffer types it covers. - Fail fast with an explicit message if the helper template resource is missing from the classpath. --- .../generator/LightProtoGenerator.java | 4 +++ .../generator/LightProtoByteBufAccess.java | 5 +-- tests/src/main/proto/proto3.proto | 6 ++++ .../lightproto/tests/TruncatedInputTest.java | 33 +++++++++++++++++-- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java index a1201e3..2f10958 100644 --- a/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java +++ b/code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoGenerator.java @@ -96,6 +96,10 @@ public static List generate(List descriptors, File ou String accessClassName = "LightProtoByteBufAccess_" + javaPackage.replace('.', '_'); try (InputStream is = LightProtoGenerator.class.getResourceAsStream( "/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java")) { + if (is == null) { + throw new IllegalStateException( + "LightProtoByteBufAccess.java template not found on the classpath"); + } String accessSource = new String(is.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8) .replace("LightProtoByteBufAccessTemplate", accessClassName); Path nettyDir = Paths.get(String.format("%s/io/netty/buffer", outputDirectory)); diff --git a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java index 420a755..7c0213e 100644 --- a/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java +++ b/code-generator/src/main/resources/io/streamnative/lightproto/generator/LightProtoByteBufAccess.java @@ -23,8 +23,9 @@ * each varint performs a single {@code readerIndex} store instead of one per byte. * *

Reads are not bounds-checked here: on truncated input a read may advance up - * to 10 bytes past the intended message limit. The generated {@code parseFrom()} - * detects that afterwards and throws, preserving the throw-on-truncated contract. + * to 9 bytes past the intended message limit (a 10-byte read whose first byte is + * inside the limit). The generated {@code parseFrom()} detects that afterwards + * and throws, preserving the throw-on-truncated contract. * *

NOTE: this class creates a split package with netty-buffer under the JPMS * module path; it is intended for classpath deployments. diff --git a/tests/src/main/proto/proto3.proto b/tests/src/main/proto/proto3.proto index 30ba20b..a1f7740 100644 --- a/tests/src/main/proto/proto3.proto +++ b/tests/src/main/proto/proto3.proto @@ -45,3 +45,9 @@ message Proto3Message { // Map map string_to_int = 16; } + +// Exercises the varint64 detection through map key/value types: the parser +// must emit the post-parse truncation check for this message. +message MapInt64Message { + map counters = 1; +} diff --git a/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java b/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java index 5a620da..8967aa1 100644 --- a/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java +++ b/tests/src/test/java/io/streamnative/lightproto/tests/TruncatedInputTest.java @@ -25,8 +25,10 @@ /** * Truncated input must throw IndexOutOfBoundsException: heap buffers fail - * safely on array bounds, and for direct buffers the unchecked readers overrun - * by at most 9 bytes before the post-parse limit check detects it. + * safely on array bounds, and for pooled direct buffers the unchecked readers + * overrun by at most 9 bytes inside the arena chunk before the post-parse + * limit check detects it. (Exactly-sized unpooled unsafe direct buffers are + * outside this guarantee — see the readVarInt64() notes in LightProtoCodec.) */ public class TruncatedInputTest { @@ -78,6 +80,33 @@ public void testTruncatedLengthDelimited() { assertThrowsOnAllBufferTypes(data); } + @Test + public void testTruncatedVarint64InMapValue() { + // MapInt64Message has no top-level 64-bit field: only the map value type + // routes through readVarInt64(), so this exercises the check emission + // driven by map key/value types. counters entry: key "k", value varint64 + // truncated at the end of the buffer. + byte[] data = {0x0A, 0x06, 0x0A, 0x01, 'k', 0x10, (byte) 0xFF, (byte) 0xFF}; + + assertThrows(IndexOutOfBoundsException.class, + () -> new MapInt64Message().parseFrom(data)); + + ByteBuf pooled = PooledByteBufAllocator.DEFAULT.directBuffer(data.length); + try { + pooled.writeBytes(data); + assertThrows(IndexOutOfBoundsException.class, + () -> new MapInt64Message().parseFrom(pooled, pooled.readableBytes())); + } finally { + pooled.release(); + } + + // Sanity: the same entry with a complete varint64 parses + byte[] valid = {0x0A, 0x06, 0x0A, 0x01, 'k', 0x10, (byte) 0xB9, 0x60}; + MapInt64Message parsed = new MapInt64Message(); + parsed.parseFrom(valid); + assertEquals(1, parsed.getCountersCount()); + } + @Test public void testValidMessageOnTightCapacityBuffer() { // A valid message on a zero-slack buffer never overruns