xiangfu0 opened a new pull request, #19273: URL: https://github.com/apache/pinot/pull/19273
A projected `attributes['key']` resolves to a single-key read against the column's forward index. #19168 gave the consuming index a scanning extractor for that, but the sealed side never got one: the chunked readers only overrode `getMapAsJsonString`, so `getMapEntryValue` fell through to the SPI default — `getMap(docId).get(key)` — and every key access on a completed segment built a full `HashMap` and ran Jackson over every entry. A table whose queries span both consuming and sealed segments paid the old cost on most of its data. This overrides the two selective hooks on `VarByteChunkForwardIndexReaderV4` (inherited by V5 and V6) and `VarByteChunkSVForwardIndexReader`, and takes four allocations and copies off the shared path: - `PreparedMapKey` pre-packs its UTF-8 into big-endian `long`s, so the frame scan compares eight key bytes per buffer read instead of one. Entries that clear the length check are the ones sharing a prefix (`k8s.pod.name` vs `k8s.node.name`), so the comparison loop is where a scan spends its time. - `JsonBuilder#appendRaw` bulk-copies instead of walking the frame one `ByteBuffer#get` at a time, which off-heap is a separate load per byte. - `getMap` and `getMapAsJsonString` on the consuming index read the off-heap view rather than copying the whole frame to a `byte[]` first. - `MutableOffHeapByteArrayStore` caches one read-only view per region, so a value view costs a single `slice` rather than `duplicate` + `slice` + `asReadOnlyBuffer`. Built on first use, so every store that is never read this way — every dictionary, every non-MAP raw column — behaves exactly as before. `deserializeMap(ByteBuffer)` now forces `BIG_ENDIAN` like the other two frame readers already did, since it is fed an off-heap view that inherits its source's order. ## Benchmarks Isolated JMH, JDK 25, values `flat`, target key `last`, µs/op: | benchmark | entries | before | after | speedup | |---|---|---|---|---| | sealed selective key | 16 | 2.216 | 0.184 | 12.0x | | sealed selective key | 64 | 8.663 | 0.630 | 13.7x | | whole-map projection | 16 | 1.046 | 0.526 | 2.0x | | whole-map projection | 64 | 4.078 | 2.012 | 2.0x | | consuming selective key | 16 | 0.229 | 0.180 | 1.27x | | consuming selective key | 64 | 0.583 | 0.424 | 1.38x | The whole-map row is the `SELECT attributes` and `LASTWITHTIME(attributes, ..., 'STRING')` path, on top of the 3.2-5.7x #19169 measured. One consuming cell (selective-as-string at 64 entries, 0.516 → 0.538) came out flat rather than faster; raising `FreqInlineSize` recovers part of it, so it reads as an inlining cliff plus benchmark layout noise at that scale rather than a real regression. ## Tests `BenchmarkMapKeyAccess` gains the sealed forward index alongside the consuming one, plus a whole-column projection case. `VarByteChunkV4Test` gains a MAP case that runs across every compression type and, through `VarByteChunkV5Test` / `VarByteChunkV6Test`, every reader version — asserting the selective readers agree with deserializing the whole frame. `MapUtilsTest` covers the word-at-a-time comparison directly: key lengths on, just under and just over the word boundary; keys agreeing over whole words and diverging only afterwards (`k8s.workload.name` vs `k8s.workload.kind`); multi-byte UTF-8 keys, whose high-bit bytes must not sign-extend into the packed word; read-only and offset buffers; and the byte-order contract. `MutableOffHeapByteArrayStoreTest` pins that views sliced from the shared region stay independent — callers on the MAP path consume position and force byte order. ## Follow-ups not in this PR - Numeric map values (`getInt` / `getLong` / `getDouble` on `MapKeyIndexReader`) still bind through Jackson. A direct byte-level decode carries real semantics around precision and overflow, so it belongs in its own change. - A query projecting K map keys scans each row's frame K times, because each key is a separate `DataSource` with its own reader and context. Extracting all requested keys in one pass per block would be a further ~Kx, but needs the key readers to share per-block state, which the column-major projection model doesn't give them cleanly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
