Skip to content

Conversation

hawkfish
Copy link
Contributor

  • Switch to using Laurens' radix partitioned column data collections.
  • Avoid recreating the RowDataCollectionScanner by adding a Reset method.

Richard Wesley added 6 commits December 2, 2022 10:57
Richard Wesley added 5 commits January 15, 2023 13:38
Adapt to the incoming data sizes instead of
guessing badly and falling back to 1024 buckets.
This was causing some performance regressions in TPC-DS.
@hawkfish
Copy link
Contributor Author

Failures appear unrelated (no window operator in the plan).

Performance improvements at both ends of the scale. For TPC-DS Q47, Q57 with small (32K) partitions the results are:

Window-Radix-Partition

For the Fannie Mae SF-4 benchmark the query time drops from 44s to 33s.

Copy link
Contributor

@lnkuiper lnkuiper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice performance improvements! I am happy to see that PartitionedColumnData is being put to use. The code looks great.

In general, I really like the new flow of the Window operator, although I don't understand how the sorting is parallelized, and how large the sorted runs are.

What I got from this so far is that during Sink the data is collected in PartitionedColumnData, and during Combine these are combined.

This is great because you can now start Finalize/GetData knowing exactly how much data you have, and how it is distributed across hash groups.

I wonder if it's better to always go for 1024 hash groups, and combine them if they're too small. This prevents re-materializing the already materialized data. However, this will slow down the appends slightly, so it's a trade-off.

However, this is very flexible. You can merge partitions until their combined size is greater than some threshold, regardless of radix bits. For example, if you merge based on radix bits, e.g., going from 4 to 3, then you will merge 0101 and 1101. However, if you always over-partition, and just merge partitions together how you see fit, you can merge 0101 with 0001, if that results in more balanced hash groups. Let's discuss this later! :)

Richard Wesley added 2 commits January 23, 2023 06:39
* Use partition size computer
* Use ColumnDataConsumer to release data faster
* Acquiesce to clangd whinging.
Copy link
Contributor

@lnkuiper lnkuiper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! I think this is ready to go

@Mytherin Mytherin merged commit 914dd8b into duckdb:master Jan 24, 2023
@hawkfish hawkfish deleted the window-partitions branch March 3, 2023 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants