[WIP] Skip index write interface - #9413
Conversation
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com> Signed-off-by: Joaquin Colacci <joaquincolacci@gmail.com>
Signed-off-by: Joaquin Colacci <joaquincolacci@gmail.com>
ce89894 to
8c2bdfd
Compare
Signed-off-by: Joaquin Colacci <joaquincolacci@gmail.com>
Signed-off-by: Joaquin Colacci <joaquincolacci@gmail.com>
Merging this PR will improve performance by 13.8%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | words_gather_scalar[65536] |
9.4 µs | 8.3 µs | +13.8% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing joacoc:skip-index-writer-integration (9a3379a) with develop (d4dc3f2)
Footnotes
-
42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
428 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩
This is just a cleanup of the bloom filter code that is being addressed in another pull request. Signed-off-by: Joaquin Colacci <joaquincolacci@gmail.com>
…t objects Initially the `dtype` was necessary when declaring the `SkipIndex/Options`, but this requires the caller to carry (or get) the target zone column `dtype` and instantiating multiple indexes, but we can defer this behaviour to the zone writer, so that when the zone writer initializes the default aggregation fns, also initializes the index aggregation fns. At this point, we can get the `dtype` from the writer column `stream`. So, this transforms the interface from: Before: `.with_skip_index(&index, dtype, session)` After: `.with_skip_index(Arc::new(index))` But to have this behaviour, I had to update `.with_skip_index()` to store indexes as `Arc<dyn SkipIndex>` instead of trying to build on call. Also, I had to add to the zone writer the list of `SkipIndex` that should init and use during writes. Still need to dedupe `skip_indexes` when pushing (left a TODO), and since skip indexes are now shared via `Arc`, I want to double check there's no internal mutability creeping in that would make that sharing unsafe. Signed-off-by: Joaquin Colacci <joaquincolacci@gmail.com>
2 participants
Rationale for this change
This PR will contain the implementation for the skip index write interface. Based on the previous PR #8933.
ZonedLayout#8901What changes are included in this PR?
So far, this will include the base changes from the previous PR, except the bloom implementation, which belongs to #9398.
What APIs are changed? Are there any user-facing changes?
Yes. This PR will contain changes that remain TBD but will reside in the zone layout writer and file strategy.
Will expand once I get a better grasp on the writer.