From 8055a18f9005e188d036a7e5005a157baa993019 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 7 Sep 2026 15:10:42 +0100 Subject: [PATCH] Fix EnumLookup hash-area sizing `EnumLookup.Big.construct()` called `findSize(byId.size())` on the line after `byId` was created and before it was populated, so the result was always `findSize(0)` == 8 regardless of enum size. Every protobuf enum therefore got an 8-slot primary + 4-slot secondary hash area. A 100-value enum pushed ~88 entries into the linearly-scanned spill area, and grew the backing arrays 4 slots at a time while building it. Sizing from `entries` gives 128 primary slots and 19 spills for the same enum. Correctness was never affected -- the spill scan finds everything -- but the cost is paid on every enum-valued field write. Adds a regression test, plus two package-private accessors on `EnumLookup.Big` so the hash-area sizing can be asserted. Co-Authored-By: Claude Opus 5 (1M context) --- .../protobuf/schema/EnumLookup.java | 7 ++- .../protobuf/schema/EnumLookupTest.java | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 protobuf/src/test/java/tools/jackson/dataformat/protobuf/schema/EnumLookupTest.java diff --git a/protobuf/src/main/java/tools/jackson/dataformat/protobuf/schema/EnumLookup.java b/protobuf/src/main/java/tools/jackson/dataformat/protobuf/schema/EnumLookup.java index bd195bf8e..c47d8ac45 100644 --- a/protobuf/src/main/java/tools/jackson/dataformat/protobuf/schema/EnumLookup.java +++ b/protobuf/src/main/java/tools/jackson/dataformat/protobuf/schema/EnumLookup.java @@ -293,7 +293,8 @@ public static Big construct(List> entries) LinkedHashMap byId = new LinkedHashMap(); // First: calculate size of primary hash area - final int size = findSize(byId.size()); + // NOTE: must size from `entries`; `byId` is still empty at this point + final int size = findSize(entries.size()); final int mask = size-1; // and allocate enough to contain primary/secondary, expand for spillovers as need be int alloc = size + (size>>1); @@ -329,6 +330,10 @@ public static Big construct(List> entries) return new Big(byId, mask, spills, keys, indices); } + // Accessors for testing of hash area sizing + int hashArea() { return _hashMask+1; } + int spillCount() { return _spillCount; } + @Override public String findEnumByIndex(int index) { return _enumsById.get(index); diff --git a/protobuf/src/test/java/tools/jackson/dataformat/protobuf/schema/EnumLookupTest.java b/protobuf/src/test/java/tools/jackson/dataformat/protobuf/schema/EnumLookupTest.java new file mode 100644 index 000000000..de8fd1e5f --- /dev/null +++ b/protobuf/src/test/java/tools/jackson/dataformat/protobuf/schema/EnumLookupTest.java @@ -0,0 +1,54 @@ +package tools.jackson.dataformat.protobuf.schema; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +/** + * Tests for {@link EnumLookup}, in particular that the "big" variant sizes its + * hash area from the actual entry count (it used to always size for 0 entries, + * pushing nearly everything into the linear-scan spill area). + */ +public class EnumLookupTest +{ + @Test + public void testBigLookupResolvesAllEntries() + { + final int count = 100; + EnumLookup lookup = EnumLookup.construct(_enumDef(count)); + for (int i = 0; i < count; ++i) { + String name = "VALUE_"+i; + assertEquals(i, lookup.findEnumIndex(name), name); + assertEquals(name, lookup.findEnumByIndex(i)); + } + assertEquals(-1, lookup.findEnumIndex("NO_SUCH_VALUE")); + assertEquals(count, lookup.getEnumValues().size()); + } + + @Test + public void testBigLookupHashAreaSizedFromEntryCount() + { + final int count = 100; + EnumLookup.Big lookup = assertInstanceOf(EnumLookup.Big.class, + EnumLookup.construct(_enumDef(count))); + // With correct sizing the primary hash area holds 128 slots; the old code + // sized for 0 entries (8 primary + 4 secondary slots), so nearly every + // entry ended up in the linearly-scanned overflow area. + assertEquals(128, lookup.hashArea()); + if (lookup.spillCount() >= (count / 2)) { + throw new AssertionError("Too many spill-over entries: "+lookup.spillCount()); + } + } + + private ProtobufEnum _enumDef(int count) { + Map values = new LinkedHashMap<>(); + for (int i = 0; i < count; ++i) { + values.put("VALUE_"+i, i); + } + return new ProtobufEnum("BigEnum", values, true); + } +}