Skip to content

[SPARK-37862][SQL] RecordBinaryComparator should fast skip the check of aligning with unaligned platform#35161

Closed
ulysses-you wants to merge 1 commit intoapache:masterfrom
ulysses-you:unaligned
Closed

[SPARK-37862][SQL] RecordBinaryComparator should fast skip the check of aligning with unaligned platform#35161
ulysses-you wants to merge 1 commit intoapache:masterfrom
ulysses-you:unaligned

Conversation

@ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

RecordBinaryComparator compare the entire row, so it need to check if the platform is unaligned. #35078 had given the perf number to show the benefits. So this PR aims to do the same thing that fast skip the check of aligning with unaligned platform.

Why are the changes needed?

Improve the performance.

Does this PR introduce any user-facing change?

no

How was this patch tested?

Pass CI. And the perf number should be same with #35078

@github-actions github-actions bot added the SQL label Jan 11, 2022
@ulysses-you
Copy link
Contributor Author

cc @srowen

@srowen
Copy link
Member

srowen commented Jan 11, 2022

Are there other instances like this?
Any chance there is an easy way to produce a benchmark result for this change too?

@ulysses-you
Copy link
Contributor Author

Are there other instances like this?

That's all. I searched and this PR is the only place should be changed

Any chance there is an easy way to produce a benchmark result for this change too?

the related code are totally same, so cannot find the reason to re-produce the bencharmk result..

@srowen
Copy link
Member

srowen commented Jan 11, 2022

I tend to agree this change is good simply for consistency. I'm wondering if it is possible this is not a win as applied in this code path, but unlikely

@srowen srowen closed this in 8ae9707 Jan 15, 2022
@srowen
Copy link
Member

srowen commented Jan 15, 2022

Merged to master

@ulysses-you
Copy link
Contributor Author

thank you @srowen

@ulysses-you ulysses-you deleted the unaligned branch January 17, 2022 01:27
dchvn pushed a commit to dchvn/spark that referenced this pull request Jan 19, 2022
…of aligning with unaligned platform

### What changes were proposed in this pull request?

`RecordBinaryComparator` compare the entire row, so it need to check if the platform is unaligned. apache#35078 had given the perf number to show the benefits. So this PR aims to do the same thing that fast skip the check of aligning with unaligned platform.

### Why are the changes needed?

Improve the performance.

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

Pass CI. And the perf number should be same with apache#35078

Closes apache#35161 from ulysses-you/unaligned.

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments