Skip to content
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

Combine low seletivity vectors generated by hash join filter #10987

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zhli1142015
Copy link
Contributor

@zhli1142015 zhli1142015 commented Sep 12, 2024

Combine low seletivity vectors generated by join filter
Problem
We found in TPCDS query72 that the join filter leads to a large number of
low-seletivity result vectors, which affects the performance of subsequent
operations.

Details:
The number of input vectors on the probe side of the corresponding join is
84,087, with a total of 743,851,486 rows (our batch size is set to 10,240). Due
to a large number of duplicate rows on the build side, the final result
inflates. The number of output vectors from the join is 2,806,054. The
corresponding join filter filters out a large portion of the results, so the
number of output rows is 1,430,253,235. This leads to the output of many
sparse vectors (the average batch row count is 504).

Solution
The original logic continues the loop to fill more rows if no rows pass the
filters. To resolve this issue, we can extend it to handle cases where only
partial rows pass the filters. We need to ensure that the indices
'outputRowMapping_' and 'outputTableRows_' are filled as much as possible until
we either reach the preferred batch size or have processed all rows in the
current input vector.

This approach will not only address the issue mentioned above but also avoid
unnecessary data copying.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 12, 2024
Copy link

netlify bot commented Sep 12, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit c691994
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6708e64aab82d60008924b91

@zhli1142015 zhli1142015 changed the title Combine low seletivity vectors generated by join filter Combine low seletivity vectors generated by hash join filter Sep 12, 2024
@zhli1142015
Copy link
Contributor Author

zhli1142015 commented Sep 12, 2024

In our experiment for TPCDS query72, we observed that due to the reduced number of vectors, there is a noticeable performance improvement in operators after the first join and the query latency improves 45% also(57s --> 31s).

Before this change
Join generates low seletivity vectors ( probe input vectors: 84087 --> output vectors: 2806054)
image

Op after above Join
image

With this changer
probe input vectors: 84087 --> output vectors: 324659
image
Op after above Join: the total probe time reduced from 2.5 min to 1.1 min
image

@zhli1142015
Copy link
Contributor Author

zhli1142015 commented Sep 12, 2024

@mbasmanova and @Yuhta , could you help to take a look?
Thanks.

@zhli1142015
Copy link
Contributor Author

CI failure is caused by #10871, thanks.

Copy link
Contributor

@Yuhta Yuhta left a comment

Choose a reason for hiding this comment

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

I think the idea is great, leave some initial comments.

Related issue #7801

velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
velox/exec/tests/HashJoinTest.cpp Outdated Show resolved Hide resolved
velox/exec/tests/HashJoinTest.cpp Outdated Show resolved Hide resolved
velox/exec/tests/HashJoinTest.cpp Outdated Show resolved Hide resolved
velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
@zhli1142015
Copy link
Contributor Author

Hello @Yuhta and @mbasmanova , could you take a look again? Thanks.

// For boolean type and if the offset is not multiple of 8, return a shifted
// copy; otherwise return a BufferView into the original buffer (with shared
// ownership of original buffer).
static BufferPtr sliceBuffer(
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this


// Intialize 'leftSemiProjectIsNull_' for null aware lft semi join.
if (isLeftSemiProjectJoin(joinType_) && nullAware_) {
leftSemiProjectIsNull_.clearAll();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do leftSemiProjectIsNull_.resize(outputTableRowsCapacity_) here so we don't need to do it in each loop? SelectivityVector::resize requires iterate over all the bits in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks.

@zhli1142015
Copy link
Contributor Author

CI failure is not related to this PR, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants