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

Add an option to disable disable measuring kudo buffer copy. #2759

Open
wants to merge 4 commits into
base: branch-25.02
Choose a base branch
from

Conversation

liurenjie1024
Copy link
Collaborator

When used in shuffle cases with large partition numbers, each call to System.nanoTime is still quite expensive compared with a small memory copy. This pr adds an option to disable this measurement and disable it by default.

@liurenjie1024
Copy link
Collaborator Author

build

@liurenjie1024
Copy link
Collaborator Author

build

@liurenjie1024
Copy link
Collaborator Author

cc @abellina @revans2

@binmahone
Copy link
Collaborator

Just out of curiosity, how expensive is System.nanoTime with relative to the actual memory copy? The code change itself looks good to me

Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

Can we please tie this into visibility of the copy buffer time metric? If this is off, but the metric still shows up on the UI it will be confusing for customers.

@liurenjie1024
Copy link
Collaborator Author

Can we please tie this into visibility of the copy buffer time metric? If this is off, but the metric still shows up on the UI it will be confusing for customers.

Hi, @revans2 I've submitted a pr to hide it in spark rapids: NVIDIA/spark-rapids#11996

The whole issue has been splitted into three steps, see NVIDIA/spark-rapids#11995

@liurenjie1024
Copy link
Collaborator Author

Just out of curiosity, how expensive is System.nanoTime with relative to the actual memory copy? The code change itself looks good to me

We have observed about 30% improvement in actual workload, where partition is quite small.

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