-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[docs] Add antipattern for nested ray.get #43184
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
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
8d9ccbc
antipattern
stephanie-wang f0223eb
Update doc/source/ray-core/patterns/nested-ray-get.rst
stephanie-wang 1685bee
Update doc/source/ray-core/patterns/nested-ray-get.rst
stephanie-wang bde9c15
Merge branch 'master' into ray-get-antipattern
jjyao 6cf6c2a
lint
stephanie-wang d7b4dcb
docs
stephanie-wang ffc9f6a
Merge remote-tracking branch 'upstream/master' into ray-get-antipattern
stephanie-wang a885fb3
Apply suggestions from code review
jjyao d3c19ec
Merge branch 'master' into ray-get-antipattern
jjyao File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
27 changes: 27 additions & 0 deletions
27
doc/source/ray-core/doc_code/anti_pattern_nested_ray_get.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# __anti_pattern_start__ | ||
import ray | ||
import time | ||
|
||
|
||
@ray.remote | ||
def f(): | ||
return 1 | ||
|
||
|
||
@ray.remote | ||
def pass_via_nested_ref(refs): | ||
print(sum(ray.get(refs))) | ||
|
||
|
||
@ray.remote | ||
def pass_via_direct_arg(*args): | ||
print(sum(args)) | ||
|
||
|
||
# Anti-pattern: Passing nested refs requires `ray.get` in a nested task. | ||
ray.get(pass_via_nested_ref.remote([f.remote() for _ in range(3)])) | ||
|
||
# Better approach: Pass refs as direct arguments. Use *args syntax to unpack | ||
# multiple arguments. | ||
ray.get(pass_via_direct_arg.remote(*[f.remote() for _ in range(3)])) | ||
# __anti_pattern_end__ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
.. _nested-ray-get: | ||
|
||
Anti-pattern: Calling ray.get on task arguments harms performance | ||
================================================================= | ||
|
||
|
||
**TLDR:** If possible, pass ``ObjectRefs`` as direct task arguments, instead of passing a list as the task argument and then calling :func:`ray.get() <ray.get>` inside the task. | ||
|
||
When a task calls ``ray.get()``, it must block until the value of the ``ObjectRef`` is ready. | ||
If all cores are already occupied, this situation can lead to a deadlock, as the task that produces the ``ObjectRef``'s value may need the caller task's resources in order to run. | ||
To handle this issue, if the caller task would block in ``ray.get()``, Ray temporarily releases the caller's CPU resources to allow the pending task to run. | ||
This behavior can harm performance and stability because the caller continues to use a process and memory to hold its stack while other tasks run. | ||
|
||
Therefore, it is always better to pass ``ObjectRefs`` as direct arguments to a task and avoid calling ``ray.get`` inside of the task, if possible. | ||
|
||
For example, in the following code, prefer the latter method of invoking the dependent task. | ||
.. literalinclude:: ../doc_code/anti_pattern_nested_ray_get.py | ||
:language: python | ||
:start-after: __anti_pattern_start__ | ||
:end-before: __anti_pattern_end__ | ||
|
||
Avoiding ``ray.get`` in nested tasks may not always be possible. Some valid reasons to call ``ray.get`` include: | ||
|
||
- :doc:`nested-tasks` | ||
- If the nested task has multiple ``ObjectRefs`` to ``ray.get``, and it wants to choose the order and number to get. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.