xiangfu0 opened a new pull request, #19171: URL: https://github.com/apache/pinot/pull/19171
> **Stacked on #19168.** This branch contains that PR's commit as its base, so the diff shows two commits — only the last one, `Decode MAP values by declared type instead of through Jackson`, belongs to this review. Happy to rebase once #19168 lands. ## Description 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. Instantiating the parser is the whole cost — roughly 60ns of the ~100ns floor a key access pays once the frame scan is excluded. ## Changes - Add `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 to `deserializeMapValue(...).toString()`. - Expose it via a `ForwardIndexReader#getMapValueAsString` hook, overridden by `VarByteSVMutableForwardIndex`. - Stop formatting numbers to a string and reparsing them in the numeric accessors. The numeric fast paths match only the exact type Jackson produces for that JSON shape — `Integer` for a small integer, `Long` for a large one, `Double` for a decimal. A broader `Number.intValue()` would silently truncate `42.7` in an `INT`-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 against `deserializeMapValue(...).toString()` — what `getString` did before: | entries | key pos | before us/op | after us/op | speedup | B/op before | B/op after | |--------:|:--------|-------------:|------------:|--------:|------------:|-----------:| | 4 | first | 0.104 | 0.040 | 2.6x | 792 | 208 | | 4 | last | 0.125 | 0.071 | 1.8x | 792 | 208 | | 16 | first | 0.100 | 0.040 | 2.5x | 792 | 208 | | 16 | last | 0.219 | 0.154 | 1.4x | 792 | 208 | | 64 | first | 0.101 | 0.040 | 2.5x | 792 | 208 | | 64 | last | 0.561 | 0.512 | 1.1x | 792 | 208 | 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 - `MapUtilsTest` 27/27, including a test asserting `deserializeMapValueAsString` matches `deserializeMapValue(...).toString()` across plain, empty, quoted, backslash, newline, unicode, int, long, double, boolean, list and nested values - `MapKeyIndexReaderTest` 2/2, `VarByteSVMutableForwardIndexTest` 3/3, `MutableOffHeapByteArrayStoreTest` 3/3 - `spotless:apply`, `license:check`, `checkstyle:check` clean on `pinot-spi`, `pinot-segment-spi`, `pinot-segment-local`, `pinot-perf` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
