Add PartitionedVector benchmark #1655
Conversation
xin-zhang2
force-pushed
the
PartitionedOutput1.0-benchmark
branch
from
18b20cf to
216a2fa
Compare
|
|
||
| void run(const RowVectorPtr& vector, int32_t numPartitions) { | ||
| folly::BenchmarkSuspender suspender; | ||
| // roundRobinPartitionFunction(vector, numPartitions, partitions_); |
There was a problem hiding this comment.
Remove the commented out line, or make the distribution a dimension as well
There was a problem hiding this comment.
We can add a non-uniform distribution to test the skew case.
| const auto vectorCopy = std::static_pointer_cast<RowVector>( | ||
| BaseVector::copy(*vector, bm->getPool())); | ||
| suspender.dismiss(); | ||
| bm->run(vectorCopy, 10000); |
There was a problem hiding this comment.
10000 partitions?
#partitions also need to be one test dimension. The value of intcan be {4, 8, 16, 32, 64, 128, 256, 512, 1024}
| } // namespace | ||
|
|
||
| class PartitionedVectorBenchmark : public VectorTestBase { | ||
| protected: |
There was a problem hiding this comment.
Move the field members to the end of the class.
| return std::dynamic_pointer_cast<RowVector>(batch); | ||
| } | ||
|
|
||
| void prepareBuffers(vector_size_t numRows, int32_t numPartitions) { |
There was a problem hiding this comment.
We can merge this function with calculatePartitionOffsets(). They are always called consecutively, and the reader has to jump into prepareBuffers() from the main logic to check what it does, because it's rather a very generic description. Merging it into calculatePartitionOffsets() would be easier for the reader, because "calculatePartitionOffsets" is self-explanatory.
| rawEndPartitionOffsets[i] = offset; | ||
| } | ||
| endPartitionOffsets_->setSize(numPartitions * sizeof(vector_size_t)); | ||
| } |
There was a problem hiding this comment.
Where did you set beginPartitionOffsets_?
| } | ||
|
|
||
| void calculatePartitionOffsets(int32_t numPartitions) { | ||
| std::vector partitionRowCounts(numPartitions, 0); |
There was a problem hiding this comment.
The local partitionRowCounts vector is not needed. We can do the calculation in place in endPartitionOffsets_.
| vector_size_t offset = 0; | ||
| for (int32_t i = 0; i < numPartitions; ++i) { | ||
| offset += partitionRowCounts[i]; | ||
| rawEndPartitionOffsets[i] = offset; |
There was a problem hiding this comment.
merge line 127 and 128 into one line.
| types.reserve(numColumns); | ||
|
|
||
| for (int i = 0; i < numColumns; ++i) { | ||
| types.push_back(typeSelection[i % typeSelection.size()]); |
There was a problem hiding this comment.
This causes repeating patterns that optimize caching.
Better shuffle types once.
| BufferPtr topRowOffsets_; | ||
| BufferPtr beginPartitionOffsets_; | ||
| BufferPtr endPartitionOffsets_; | ||
| BufferPtr swappingBuffer_; |
There was a problem hiding this comment.
Never allocated or sized before use.
| folly::runBenchmarks(); | ||
| bm.reset(); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Can you please add the run output as a comment at the end of the file?
2 participants
No description provided.