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); + } +}