Decode MAP values by declared type instead of through Jackson - #19171
Open
xiangfu0 wants to merge 3 commits into
Open
Decode MAP values by declared type instead of through Jackson#19171xiangfu0 wants to merge 3 commits into
xiangfu0 wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes MAP key access in Pinot’s forward-index path by decoding MAP values according to the declared type and avoiding unnecessary Jackson parsing, especially for STRING-valued MAP entries.
Changes:
- Added selective MAP value extraction APIs (
deserializeMapValue*,ForwardIndexReader#getMapValue*) to avoid deserializing full maps when only one key is needed. - Added a fast-path to decode plain JSON string literals without invoking Jackson, while preserving exact rendering for non-plain-string shapes via fallback.
- Reduced numeric accessor overhead by avoiding unnecessary string round-trips when the deserialized value is already the expected boxed type.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pinot-spi/src/main/java/org/apache/pinot/spi/utils/MapUtils.java | Adds selective MAP value extraction and a plain-string fast-path for string rendering. |
| pinot-spi/src/test/java/org/apache/pinot/spi/utils/MapUtilsTest.java | Adds coverage for selective extraction, UTF-8 key matching, big-endian enforcement, and string-rendering equivalence. |
| pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/ForwardIndexReader.java | Adds default hooks for selective per-key MAP access and string rendering. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/map/MapKeyIndexReader.java | Switches MAP key accessors to use the selective APIs and adds numeric fast-paths. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/forward/VarByteSVMutableForwardIndex.java | Implements selective MAP key access by decoding directly from an off-heap byte-buffer view. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/MutableOffHeapByteArrayStore.java | Exposes a zero-copy ByteBuffer view of stored values for selective decoding. |
| pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/map/MapKeyIndexReaderTest.java | New tests ensuring behavior parity between selective and full-map readers. |
| pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/forward/mutable/VarByteSVMutableForwardIndexTest.java | Adds tests covering selective MAP key reads from the mutable forward index. |
| pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkMapKeyAccess.java | Adds a JMH benchmark to quantify full-map vs selective lookup and string fast-path behavior. |
Comment on lines
79
to
82
| public float getFloat(int docId, ForwardIndexReaderContext context) { | ||
| return Float.parseFloat(extractMapValue(docId, context, _keyName).toString()); | ||
| Object value = extractMapValue(docId, context, _keyName); | ||
| return value instanceof Float ? (Float) value : Float.parseFloat(value.toString()); | ||
| } |
Comment on lines
+291
to
+300
| int valueLength = byteBuffer.getInt(); | ||
| if (!matches) { | ||
| skip(byteBuffer, valueLength); | ||
| continue; | ||
| } | ||
| // Keys within a frame are unique - the write path iterates a Map - so the first match is the only match and | ||
| // the remaining entries never need to be scanned. | ||
| byte[] valueBytes = new byte[valueLength]; | ||
| byteBuffer.get(valueBytes); | ||
| return valueBytes; |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19171 +/- ##
============================================
- Coverage 66.65% 66.64% -0.01%
Complexity 1423 1423
============================================
Files 3443 3443
Lines 218632 218711 +79
Branches 34793 34817 +24
============================================
+ Hits 145726 145769 +43
- Misses 61192 61215 +23
- Partials 11714 11727 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xiangfu0
force-pushed
the
agent/map-value-typed-decode
branch
from
August 7, 2026 09:28
d43c396 to
2c4b2a1
Compare
deserializeMapValue walked every key one relative get at a time - even after a mismatch was already certain - purely to advance the position, and kept scanning the frame after the match was found. Compare through absolute gets so a length mismatch or a differing byte skips the rest of the key outright, and return on the first match. Keys within a frame are unique because the write path iterates a Map, so the first match is the only match. Bounds-check the key length up front so the absolute gets are provably in range and a truncated frame still surfaces as BufferUnderflowException. Isolated JMH, flat string values, fixed-length dotted keys, JDK 25: entries key full map before after 4 first 0.556 0.166 0.114 us/op 16 first 2.163 0.360 0.112 us/op 64 first 8.886 1.074 0.118 us/op 64 last 8.763 1.245 0.593 us/op First-key lookup no longer scales with map size. Allocation is unchanged at 856 B/op versus 62792 B/op for the full-map path. Also cover MapKeyIndexReader, which had no test despite being the caller that changed, over both a reader that overrides getMapValue and one that inherits the default, plus non-ASCII keys and a little-endian buffer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A MAP column with a STRING value type stores "pinot-server" in the frame, and MapKeyIndexReader#getString parsed that into a String only to call toString() on it. The parser instantiation is the whole cost: it is about 60ns of the ~100ns floor a key access pays once the scan is excluded. Add MapUtils#deserializeMapValueAsString, which decodes a JSON string literal carrying no escapes directly and hands everything else - numbers, booleans, objects, arrays - to Jackson so the rendering stays identical. Expose it through a ForwardIndexReader#getMapValueAsString hook that the mutable forward index overrides. Also stop formatting numbers to a string and reparsing them. The numeric accessors fast-path only the exact type Jackson produces for that JSON shape - Integer for a small integer, Long for a large one, Double for a decimal - so a value that does not match the declared type still fails the way it did before rather than being silently coerced. Isolated JMH, JDK 25, against deserializeMapValue(...).toString(): entries key before after B/op before B/op after 4 first 0.104 0.040 792 208 16 first 0.100 0.040 792 208 64 first 0.101 0.040 792 208 16 last 0.219 0.154 792 208 64 last 0.561 0.512 792 208 The fixed per-access cost drops 2.5x and allocation 3.8x. At 64/last the gain is only 1.1x because the frame scan, not Jackson, dominates there. Values that are not plain strings take the Jackson fallback and measure 3-8% slower with identical allocation, the cost of the failed plain-string check. Such maps would not normally resolve through getString. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
xiangfu0
force-pushed
the
agent/map-value-typed-decode
branch
from
August 8, 2026 09:36
2c4b2a1 to
a6a20a8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A
MAPcolumn with aSTRINGvalue type stores"pinot-server"in the frame, andMapKeyIndexReader#getStringparsed that into aStringonly to calltoString()on it. Instantiating the parser is the whole cost — roughly 60ns of the ~100ns floor a key access pays once the frame scan is excluded.Changes
MapUtils#deserializeMapValueAsString. A JSON string literal carrying no escapes is decoded directly; everything else — numbers, booleans, objects, arrays — still goes through Jackson so the rendering is identical todeserializeMapValue(...).toString().ForwardIndexReader#getMapValueAsStringhook, overridden byVarByteSVMutableForwardIndex.The numeric fast paths match only the exact type Jackson produces for that JSON shape —
Integerfor a small integer,Longfor a large one,Doublefor a decimal. A broaderNumber.intValue()would silently truncate42.7in anINT-declared map where today it throws, and silently coercing a type mismatch is worse than failing, so anything else still takes the string round trip.Performance
Isolated JMH (
BenchmarkMapKeyAccess), JDK 25, 2 forks x 5x1s,-prof gc, measured againstdeserializeMapValue(...).toString()— whatgetStringdid before:The fixed per-access cost drops 2.5x and allocation 3.8x, flat across map sizes. At 64/last the gain narrows to 1.1x because the frame scan, not Jackson, dominates there.
Regression to be aware of: values that are not plain strings take the Jackson fallback and measure 3-8% slower with identical allocation (1528 B/op both ways) — the cost of the failed plain-string check plus the extra call boundary. It is consistent across all six nested combinations rather than noise. Maps with object values would not normally resolve through
getString, but the cost is real.This is an isolated forward-index measurement, not an end-to-end query latency result.
Validation
MapUtilsTest27/27, including a test assertingdeserializeMapValueAsStringmatchesdeserializeMapValue(...).toString()across plain, empty, quoted, backslash, newline, unicode, int, long, double, boolean, list and nested valuesMapKeyIndexReaderTest2/2,VarByteSVMutableForwardIndexTest3/3,MutableOffHeapByteArrayStoreTest3/3spotless:apply,license:check,checkstyle:checkclean onpinot-spi,pinot-segment-spi,pinot-segment-local,pinot-perf