Skip to content

Issue #7809: Segment Tree Performance #7831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 7, 2023

Conversation

hawkfish
Copy link
Contributor

@hawkfish hawkfish commented Jun 5, 2023

  • Fix some obvious low hanging fruit with window segment trees.
  • Use bitwise operations to Slice ValidityMask in place when the target is aligned.
  • Fix case when ragged end is larger than the head.
  • Initialise combine buffer BEFORE it is used...
  • Implement Vector::SliceInPlace and use for windowing.

Test case performance:

Change Time
Baseline 33.886
Update Vector 32.482
Levels Vector 21.173
Slice Aligned 19.432
SliceInPlace 15.759

Richard Wesley added 6 commits June 3, 2023 13:13
Fix some obvious low hanging fruit with window segment trees.
Use bitwise operations to Slice ValidityMask in place
when the target is aligned. Drops from 21.2s to 19.7s.
Fix case when ragged end is larger than the head.
Initialise combine buffer BEFORE it is used...
Implement Vector::SliceInPlace and use for windowing.
@hawkfish hawkfish requested a review from Mytherin June 5, 2023 15:58
@hawkfish
Copy link
Contributor Author

hawkfish commented Jun 5, 2023

That perf regression has nothing to do with the window code, and it was passing on my fork, so I think it is spurious?

@Mytherin
Copy link
Collaborator

Mytherin commented Jun 5, 2023

The performance regression is caused by #7697, and will unfortunately persist until we merge feature into master (or until we change the CI not to compare feature to master).

@Mytherin Mytherin merged commit 3a0c1a9 into duckdb:feature Jun 7, 2023
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