Skip to content

[tune/execution][rfc] Cache ready futures in RayTrialExecutor - #32093

Merged
krfricke merged 7 commits into
ray-project:masterfrom
krfricke:tune/cache-events
Feb 1, 2023
Merged

[tune/execution][rfc] Cache ready futures in RayTrialExecutor#32093
krfricke merged 7 commits into
ray-project:masterfrom
krfricke:tune/cache-events

Conversation

@krfricke

Copy link
Copy Markdown
Contributor

Signed-off-by: Kai Fricke kai@anyscale.com

Why are these changes needed?

We currently resolve futures one-by-one in Ray Tune, and query Ray core for the ready status of future multiple times. Instead, we can also cache ready events and yield them if cached elements exist. This can improve performance: In tune_scalability_result_cluster_throughput this improved performance by ~2-3%.

We will always re-query Ray if we expect a resource to be ready.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Kai Fricke added 2 commits January 30, 2023 17:53
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>

@xwjiang2010 xwjiang2010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Comment thread python/ray/tune/execution/ray_trial_executor.py
Signed-off-by: Kai Fricke <kai@anyscale.com>
@krfricke
krfricke merged commit 6e39b2e into ray-project:master Feb 1, 2023
@krfricke
krfricke deleted the tune/cache-events branch February 1, 2023 21:16
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
…oject#32093)

We currently resolve futures one-by-one in Ray Tune, and query Ray core for the ready status of future multiple times. Instead, we can also cache ready events and yield them if cached elements exist. This can improve performance: In tune_scalability_result_cluster_throughput this improved performance by ~2-3%.

We will always re-query Ray if we expect a resource to be ready.

Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
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.

2 participants