apache / apache/pinot

Benchmark setColumn() in DataTableBuilder and write all values one by one instead of using rowId and colId to position if need be

Open
#6,720 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
6.1k
Forks
1.5k
Avg merge
1d 21h
Merged PRs (30d)
189

Description

As pointed by a TODO in DataTableBuilder: https://github.com/apache/incubator-pinot/blob/e892cb2942f1f41ac7056d977eeca0e5b22897d0/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java#L77

Currently, `setColumn()` use rowId&colId to first locate the offset of ByteBuffer, the write the actual value to the offset. This allowed us to set the value for a given colId, but have a lot of overhead of calling `ByteBuffer.position()`. In practice, we often build a data table row in a bulk way, which means we can write all values of a row one by one, without calling `ByteBuffer.position()` for each column.

We need to benchmark those two approaches (it's possible that the overhead of calling `ByteBuffer.position()` is negligible). If the bulk way is better, will send a PR to address it.

Contributor guide

Open the contributing guide

Research direction

Start at pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java, especially the TODO around line 77 and the setColumn() implementation. Benchmark the existing rowId/colId and ByteBuffer.position() approach against writing row values sequentially. Done means recording whether the bulk approach is measurably faster and determining whether the result justifies a follow-up change.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.