-
Notifications
You must be signed in to change notification settings - Fork 68
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
Improve kudo concat performance. #2915
Improve kudo concat performance. #2915
Conversation
Signed-off-by: liurenjie1024 <[email protected]>
build |
build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main question is how much faster did this make shuffle? This is a performance improvement, but I don't see any benchmark results for how much better it is.
src/main/java/com/nvidia/spark/rapids/jni/schema/SchemaVisitor2.java
Outdated
Show resolved
Hide resolved
src/main/java/com/nvidia/spark/rapids/jni/kudo/KudoHostMergeResult.java
Outdated
Show resolved
Hide resolved
src/main/java/com/nvidia/spark/rapids/jni/kudo/KudoSerializer.java
Outdated
Show resolved
Hide resolved
src/main/java/com/nvidia/spark/rapids/jni/kudo/KudoTableMerger.java
Outdated
Show resolved
Hide resolved
src/main/java/com/nvidia/spark/rapids/jni/kudo/MergedInfoCalc.java
Outdated
Show resolved
Hide resolved
build |
I did some local micro benchmark about concating, and it shows 20% performance improvement, mainly introduced by optimizing concat validity buffer. My local branch shows nds result about 2% perf improvement. |
The code is in fact ready for review, I'm marking it as draft since I'm running integration test to verify it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I would prefer better names than caseOne, caseTwo, and caseThree, but it is documented and probably good enoguh.
build |
Run integeration tests offline and it passed, merge this now. |
Closes #2913
Closes #2579
This pr improves kudo concatting performace by following approaches:
SchemaVisitor
interface to avlid unnecessary allocation.KudoTableMerger
andMergeInfoCalc
with arrays to avoid unnecessary allocation.