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

CPU-GPU sync contrary to comment #46

Closed
Ilykuleshov opened this issue Oct 2, 2024 · 4 comments · Fixed by #48
Closed

CPU-GPU sync contrary to comment #46

Ilykuleshov opened this issue Oct 2, 2024 · 4 comments · Fixed by #48

Comments

@Ilykuleshov
Copy link
Collaborator

Hi, sorry to bother you again. Seems that this line shouldn't be there, at least according to the comment above. I was able to achieve a significant speed-up by removing this if.

@martenlienen
Copy link
Owner

You are right that this is inconsistent with the comment. If I remember correctly, this might come down to the problem being solved. Timing this is also non-trivial. If you come to a conclusion about which way is faster, we can either fix the comment or make the code behave as described again. Thanks for looking into this!

@martenlienen
Copy link
Owner

I have added you as a collaborator to the repository :)

@Ilykuleshov
Copy link
Collaborator Author

Ilykuleshov commented Oct 3, 2024

I think I know what's going on here: to_be_evaluated.any() indeed causes a blocking CPU-GPU sync point, but:

  1. A sync is necessary at each evaluation point anyway, since we can't know how many evaluation points have been passed (it happens at nonzero, see Certain operations cause implicity sync-points pytorch/pytorch#12461).
  2. Without evaluation points, this code is unreachable, so no syncs happen.

Theoretically, this could be an issue for sparse evaluation points, but in my experiments I couldn't achieve a speed-up by deferring this sync, so I'm just going to sum all this up in the comment, referencing this issue, and leave things as-is.

@martenlienen
Copy link
Owner

Good catch that there is a sync point happening anyway!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants