From 4ef757f785fff3fb330860194d8afa7ef4fcb1e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Rou=C3=A9l?= Date: Sun, 19 Apr 2026 19:33:43 +0200 Subject: [PATCH 1/3] GH-3501: Speed up `DictionaryValuesWriter.shouldFallBack()` --- .../dictionary/DictionaryValuesWriter.java | 27 +++++++++++++++++-- .../values/dictionary/TestDictionary.java | 23 ++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java b/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java index f4ed350e3a..16ac44a9e3 100644 --- a/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java +++ b/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java @@ -73,6 +73,10 @@ public abstract class DictionaryValuesWriter extends ValuesWriter implements Req /* will become true if the dictionary becomes too big */ protected boolean dictionaryTooBig; + /* set to true when the dictionary exceeds maxDictionaryByteSize or MAX_DICTIONARY_ENTRIES, + * checked by shouldFallBack() to avoid repeated virtual dispatch to getDictionarySize() on every write */ + private boolean dictionarySizeExceeded; + /* current size in bytes the dictionary will take once serialized */ protected long dictionaryByteSize; @@ -115,8 +119,20 @@ protected DictionaryPage dictPage(ValuesWriter dictPageWriter) { @Override public boolean shouldFallBack() { - // if the dictionary reaches the max byte size or the values can not be encoded on 4 bytes anymore. - return dictionaryByteSize > maxDictionaryByteSize || getDictionarySize() > MAX_DICTIONARY_ENTRIES; + return dictionarySizeExceeded; + } + + /** + * Called by subclass write methods after adding a new dictionary entry to check if the dictionary + * has exceeded its size limits. This avoids the per-value virtual dispatch overhead of calling + * getDictionarySize() on every write -- the check only runs when a new entry is actually added. + * + * @param newDictionarySize the current dictionary size after adding the new entry + */ + protected void checkDictionarySizeLimit(int newDictionarySize) { + if (dictionaryByteSize > maxDictionaryByteSize || newDictionarySize > MAX_DICTIONARY_ENTRIES) { + dictionarySizeExceeded = true; + } } @Override @@ -203,6 +219,7 @@ public void resetDictionary() { lastUsedDictionarySize = 0; dictionaryTooBig = false; dictionaryByteSize = 0; + dictionarySizeExceeded = false; clearDictionaryContent(); } @@ -245,6 +262,7 @@ public void writeBytes(Binary v) { binaryDictionaryContent.put(v.copy(), id); // length as int (4 bytes) + actual bytes dictionaryByteSize += 4L + v.length(); + checkDictionarySizeLimit(id + 1); } encodedValues.add(id); } @@ -315,6 +333,7 @@ public void writeBytes(Binary value) { id = binaryDictionaryContent.size(); binaryDictionaryContent.put(value.copy(), id); dictionaryByteSize += length; + checkDictionarySizeLimit(id + 1); } encodedValues.add(id); } @@ -359,6 +378,7 @@ public void writeLong(long v) { id = longDictionaryContent.size(); longDictionaryContent.put(v, id); dictionaryByteSize += 8; + checkDictionarySizeLimit(id + 1); } encodedValues.add(id); } @@ -431,6 +451,7 @@ public void writeDouble(double v) { id = doubleDictionaryContent.size(); doubleDictionaryContent.put(bits, id); dictionaryByteSize += 8; + checkDictionarySizeLimit(id + 1); } encodedValues.add(id); } @@ -505,6 +526,7 @@ public void writeInteger(int v) { id = intDictionaryContent.size(); intDictionaryContent.put(v, id); dictionaryByteSize += 4; + checkDictionarySizeLimit(id + 1); } encodedValues.add(id); } @@ -578,6 +600,7 @@ public void writeFloat(float v) { id = floatDictionaryContent.size(); floatDictionaryContent.put(bits, id); dictionaryByteSize += 4; + checkDictionarySizeLimit(id + 1); } encodedValues.add(id); } diff --git a/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java b/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java index 44f90d901e..d0159484a8 100644 --- a/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java +++ b/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java @@ -827,6 +827,29 @@ public void testZeroValues() throws IOException { } } + @Test + public void testCheckDictionarySizeLimitExceedsByEntryCount() { + // Use a large maxDictionaryByteSize so only the entry-count limit can trigger + int maxDictionaryByteSize = Integer.MAX_VALUE; + try (PlainIntegerDictionaryValuesWriter writer = new PlainIntegerDictionaryValuesWriter( + maxDictionaryByteSize, PLAIN_DICTIONARY, PLAIN_DICTIONARY, allocator)) { + + assertFalse("should not fall back initially", writer.shouldFallBack()); + + // At the limit (Integer.MAX_VALUE - 1 entries): should NOT trigger + writer.checkDictionarySizeLimit(Integer.MAX_VALUE - 1); + assertFalse("should not fall back when entry count equals MAX_DICTIONARY_ENTRIES", writer.shouldFallBack()); + + // Exceeding the limit (Integer.MAX_VALUE entries): should trigger + writer.checkDictionarySizeLimit(Integer.MAX_VALUE); + assertTrue("should fall back when entry count exceeds MAX_DICTIONARY_ENTRIES", writer.shouldFallBack()); + + // resetDictionary clears the flag + writer.resetDictionary(); + assertFalse("should not fall back after resetDictionary", writer.shouldFallBack()); + } + } + @Test public void testBooleanDictionary() throws IOException { // Create a dictionary page with boolean values (false, true) From b72905372b50d725987be414faf248720ce29505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Rou=C3=A9l?= Date: Tue, 16 Jun 2026 17:51:59 +0200 Subject: [PATCH 2/3] Update DictionaryValuesWriter.java --- .../column/values/dictionary/DictionaryValuesWriter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java b/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java index 16ac44a9e3..62cfff5a7b 100644 --- a/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java +++ b/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java @@ -129,7 +129,7 @@ public boolean shouldFallBack() { * * @param newDictionarySize the current dictionary size after adding the new entry */ - protected void checkDictionarySizeLimit(int newDictionarySize) { + void checkDictionarySizeLimit(int newDictionarySize) { if (dictionaryByteSize > maxDictionaryByteSize || newDictionarySize > MAX_DICTIONARY_ENTRIES) { dictionarySizeExceeded = true; } From bee244e72e9c73c3ba567fe59e80c96805a09157 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Rou=C3=A9l?= Date: Fri, 30 Oct 2026 13:03:23 +0100 Subject: [PATCH 3/3] Update TestDictionary.java --- .../column/values/dictionary/TestDictionary.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java b/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java index d0159484a8..677cdd7be8 100644 --- a/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java +++ b/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java @@ -834,19 +834,23 @@ public void testCheckDictionarySizeLimitExceedsByEntryCount() { try (PlainIntegerDictionaryValuesWriter writer = new PlainIntegerDictionaryValuesWriter( maxDictionaryByteSize, PLAIN_DICTIONARY, PLAIN_DICTIONARY, allocator)) { - assertFalse("should not fall back initially", writer.shouldFallBack()); + assertThat(writer.shouldFallBack()).as("should not fall back initially").isFalse(); // At the limit (Integer.MAX_VALUE - 1 entries): should NOT trigger writer.checkDictionarySizeLimit(Integer.MAX_VALUE - 1); - assertFalse("should not fall back when entry count equals MAX_DICTIONARY_ENTRIES", writer.shouldFallBack()); + assertThat(writer.shouldFallBack()) + .as("should not fall back when entry count equals MAX_DICTIONARY_ENTRIES") + .isFalse(); // Exceeding the limit (Integer.MAX_VALUE entries): should trigger writer.checkDictionarySizeLimit(Integer.MAX_VALUE); - assertTrue("should fall back when entry count exceeds MAX_DICTIONARY_ENTRIES", writer.shouldFallBack()); + assertThat(writer.shouldFallBack()) + .as("should fall back when entry count exceeds MAX_DICTIONARY_ENTRIES") + .isTrue(); // resetDictionary clears the flag writer.resetDictionary(); - assertFalse("should not fall back after resetDictionary", writer.shouldFallBack()); + assertThat(writer.shouldFallBack()).as("should not fall back after resetDictionary").isFalse(); } }