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

[Data] Performance regression in iter_batches prefetching #35521

Closed
amogkam opened this issue May 18, 2023 · 2 comments · Fixed by #35568
Closed

[Data] Performance regression in iter_batches prefetching #35521

amogkam opened this issue May 18, 2023 · 2 comments · Fixed by #35568
Assignees
Labels
data Ray Data-related issues P0 Issues that should be fixed in short order Ray 2.6

Comments

@amogkam
Copy link
Contributor

amogkam commented May 18, 2023

Screen Shot 2023-05-18 at 2 26 43 PM

Performance regression with prefetch_batches=1 no longer having any performance improvement compared to prefetch_batches=0.

This happened between 4/28-5/1.

Last ok commit: 23d7e9f
Failing commit: 7071a4f

Likely culprit seems to be #34871

@amogkam amogkam added data Ray Data-related issues P0 Issues that should be fixed in short order labels May 18, 2023
@amogkam amogkam self-assigned this May 18, 2023
@amogkam amogkam added the release-blocker P0 Issue that blocks the release label May 19, 2023
@amogkam
Copy link
Contributor Author

amogkam commented May 19, 2023

Confirmed that reverting #34871 fixes the regression: #35522 (comment)

@amogkam amogkam added core Issues that should be addressed in Ray Core and removed data Ray Data-related issues labels May 19, 2023
@amogkam amogkam assigned scv119 and xieus and unassigned amogkam May 19, 2023
@amogkam amogkam added Ray 2.6 and removed release-blocker P0 Issue that blocks the release labels May 19, 2023
@amogkam amogkam added data Ray Data-related issues and removed core Issues that should be addressed in Ray Core labels May 31, 2023
@amogkam amogkam unassigned scv119 and xieus May 31, 2023
@amogkam
Copy link
Contributor Author

amogkam commented May 31, 2023

After discussion with @ericl and @raulchen looks like this is owned by Ray Data team, not core team as previously thought.

raulchen added a commit that referenced this issue Jun 1, 2023
## Why are these changes needed?

`WaitBlockPrefetcher` will blockingly wait for the first block. When prefetch size is small, this can cause latency on the critical path. This PR moves the wait to a background thread.

## Related issue number

closes #35521
arvind-chandra pushed a commit to lmco/ray that referenced this issue Aug 31, 2023
## Why are these changes needed?

`WaitBlockPrefetcher` will blockingly wait for the first block. When prefetch size is small, this can cause latency on the critical path. This PR moves the wait to a background thread.

## Related issue number

closes ray-project#35521

Signed-off-by: e428265 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data Ray Data-related issues P0 Issues that should be fixed in short order Ray 2.6
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants