From a30179079b9282afce34ba7cfc59e8f6c4070258 Mon Sep 17 00:00:00 2001 From: Mohamed Ashraf Date: Thu, 23 Apr 2026 17:33:21 +0000 Subject: [PATCH] perf(parquet): hybrid 2-byte unroll + tail loop for readUleb128Int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the fully-unrolled 5-branch implementation with a hybrid: - Fast straight-line paths for 1-byte and 2-byte values (the most common cases: small dictionary indices, page/group counts, bit-width markers). - Tight loop for 3-, 4- and 5-byte values. The prior unrolled implementation pays a deep branch-chain cost on longer ULEB128 sequences, where a simple loop body is faster. This change keeps the straight-line code for the common short values while collapsing the rare long values into a loop that the JIT can optimize more effectively. BenchmarkReadUleb128Int (JDK 25, 3 forks x 10 warmup + 15 measurement x 500ms, throughput in ops/ms, 99.9% CIs, non-overlapping for every row): Config (maxValue, size) Before After Change (20000, 100) 3929.8 ± 34.6 4202.4 ± 43.0 +6.9% (20000, 1000) 214.2 ± 2.2 218.6 ± 2.2 +2.1% (4000000, 100) 3247.4 ± 22.5 3403.0 ± 32.8 +4.8% (4000000, 1000) 165.5 ± 1.8 208.1 ± 2.4 +25.7% The in-benchmark readUleb128IntLoop control (unchanged code path) shows noise-only variation (-2.2% to 0.0%), confirming the measurement infrastructure is stable. Verification: - ./mvnw -pl lib/trino-parquet test: 67242 tests, 0 failures. - ./mvnw -pl lib/trino-parquet validate: checkstyle and modernizer clean. - ./mvnw -pl lib/trino-parquet -Perrorprone-compiler compile: clean. - TestParquetReaderUtilsBenchmarks (random-input roundtrip): pass. - TestRleBitPackingDecoderBenchmark (primary consumer): pass. Co-Authored-By: Claude Opus 4.7 --- .../io/trino/parquet/ParquetReaderUtils.java | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetReaderUtils.java b/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetReaderUtils.java index a608955e9b8d..77ae5bcf40ab 100644 --- a/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetReaderUtils.java +++ b/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetReaderUtils.java @@ -52,35 +52,35 @@ public static int readUleb128Int(SimpleSliceInputStream input) { byte[] inputBytes = input.getByteArray(); int offset = input.getByteArrayOffset(); - // Manual loop unrolling shows improvements in BenchmarkReadUleb128Int - int inputByte = inputBytes[offset]; - int value = inputByte & 0x7F; - if ((inputByte & 0x80) == 0) { + // Fast paths for 1-byte and 2-byte values (most common for small dictionary indices + // and page/group counts). For 3+ byte values, fall through to a tight loop. This beats + // the fully unrolled 5-branch version on long input sequences (where the unrolled + // version's branch chain depth dominates), while preserving straight-line code for + // the common short values (where the unrolled version wins). + byte b0 = inputBytes[offset]; + if ((b0 & 0x80) == 0) { input.skip(1); - return value; + return b0; } - inputByte = inputBytes[offset + 1]; - value |= (inputByte & 0x7F) << 7; - if ((inputByte & 0x80) == 0) { + byte b1 = inputBytes[offset + 1]; + int value = (b0 & 0x7F) | ((b1 & 0x7F) << 7); + if ((b1 & 0x80) == 0) { input.skip(2); return value; } - inputByte = inputBytes[offset + 2]; - value |= (inputByte & 0x7F) << 14; - if ((inputByte & 0x80) == 0) { - input.skip(3); - return value; - } - inputByte = inputBytes[offset + 3]; - value |= (inputByte & 0x7F) << 21; - if ((inputByte & 0x80) == 0) { - input.skip(4); - return value; + int consumed = 2; + int shift = 14; + byte b; + do { + b = inputBytes[offset + consumed]; + value |= (b & 0x7F) << shift; + shift += 7; + consumed++; } - inputByte = inputBytes[offset + 4]; - verify((inputByte & 0x80) == 0, "ULEB128 variable-width integer should not be longer than 5 bytes"); - input.skip(5); - return value | inputByte << 28; + while ((b & 0x80) != 0 && shift < 35); + verify((b & 0x80) == 0, "ULEB128 variable-width integer should not be longer than 5 bytes"); + input.skip(consumed); + return value; } public static long readUleb128Long(SimpleSliceInputStream input)