-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Make execution plan/blocklist aware of the memory ownership and who runs the plan #26650
Merged
Merged
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
6414e58
Make execution stage/blocklist aware if it's run by Dataset or Datase…
103ebde
Merge branch 'master' of https://github.com/ray-project/ray into pipe…
9a8081c
fix split
65b7aeb
fix and test
445d38d
[AIR/Docs] Add Predictor Docs (#25833)
amogkam 18f46d6
spread-split-tasks (#26638)
scv119 41295f4
[air] Add _max_cpu_fraction_per_node to ScalingConfig and documentati…
ericl 3d4af4a
[RLlib] improved unittests for dataset_reader and fixed bugs (#26458)
kouroshHakha e999300
[RLlib]: Fix OPE trainables (#26279)
Rohan138 215d928
[Datasets] [Local Shuffle - 1/N] Add local shuffling option. (#26094)
clarkzinzow 9faca47
[air] Add a warning if no CPUs are reserved for dataset execution (#2…
ericl d71114c
added summary why and when to use bulk vs streaming data ingest (#26637)
dmatrix 8594d20
[AIR][CUJ] Make distributing training benchmark at silver tier (#26640)
jiaodong 95cfc7d
[Data][split] use split_at_indices for equal split without locality h…
scv119 78783dc
[Air][Data] Don't promote locality_hints for split (#26647)
scv119 3bb72c1
Add Tao as Java worker code owner. (#26596)
jovany-wang 0d9216c
[RLlib] Add/reorder Args of Prioritized/MixIn MultiAgentReplayBuffer.…
ArturNiederfahrenhorst 5608c82
[Serve] Remove EXPERIMENTAL inside the comments for user config (#26521)
sihanwang41 16322fa
Fix windows buildkite (#26615)
jjyao 2d92130
fix split
9e71304
fix and test
5f8b8fd
Merge branch 'master' of https://github.com/ray-project/ray into pipe…
060d7f7
Merge branch 'pipelineplan' of https://github.com/jianoaix/ray into p…
28ccc5e
test
9c32ae0
consumable v.s. non-consumable blocklist
jianoaix 4b83ac1
lint; test
jianoaix 84faf33
feedback: naming/concept
jianoaix 397308f
Merge branch 'master' of https://github.com/ray-project/ray into pipe…
jianoaix b4db0e6
feedback: run_by_pipeline->run_by_consumer
jianoaix 9c41a71
feedback: make owned_by_consumer required keyword-only arg
jianoaix 13c9ed6
feedback: make run_by_consumer required keyword-only arg; and make th…
jianoaix 5d1ca73
typo
jianoaix 0942498
undo no-op changes on stage_impl.py
jianoaix 91b0b2f
undo continued
jianoaix 6d3e9aa
fix
jianoaix 01f3da9
feedback: document transformation v.s. consumption concept
jianoaix 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
This file contains 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 |
---|---|---|
|
@@ -15,19 +15,30 @@ class BlockList: | |
change after execution due to block splitting. | ||
""" | ||
|
||
def __init__(self, blocks: List[ObjectRef[Block]], metadata: List[BlockMetadata]): | ||
def __init__( | ||
self, | ||
blocks: List[ObjectRef[Block]], | ||
metadata: List[BlockMetadata], | ||
*, | ||
owned_by_consumer: bool, | ||
): | ||
assert len(blocks) == len(metadata), (blocks, metadata) | ||
self._blocks: List[ObjectRef[Block]] = blocks | ||
self._num_blocks = len(self._blocks) | ||
self._metadata: List[BlockMetadata] = metadata | ||
# Whether the block list is owned by consuming APIs, and if so it can be | ||
# eagerly deleted after read by the consumer. | ||
self._owned_by_consumer = owned_by_consumer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shall we clearly define what is consumer in one place, e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
def get_metadata(self, fetch_if_missing: bool = False) -> List[BlockMetadata]: | ||
"""Get the metadata for all blocks.""" | ||
return self._metadata.copy() | ||
|
||
def copy(self) -> "BlockList": | ||
"""Perform a shallow copy of this BlockList.""" | ||
return BlockList(self._blocks, self._metadata) | ||
return BlockList( | ||
self._blocks, self._metadata, owned_by_consumer=self._owned_by_consumer | ||
) | ||
|
||
def clear(self) -> None: | ||
"""Erase references to the tasks tracked by the BlockList.""" | ||
|
@@ -57,7 +68,11 @@ def split(self, split_size: int) -> List["BlockList"]: | |
meta = np.array_split(self._metadata, num_splits) | ||
output = [] | ||
for b, m in zip(blocks, meta): | ||
output.append(BlockList(b.tolist(), m.tolist())) | ||
output.append( | ||
BlockList( | ||
b.tolist(), m.tolist(), owned_by_consumer=self._owned_by_consumer | ||
) | ||
) | ||
return output | ||
|
||
def split_by_bytes(self, bytes_per_split: int) -> List["BlockList"]: | ||
|
@@ -78,15 +93,23 @@ def split_by_bytes(self, bytes_per_split: int) -> List["BlockList"]: | |
) | ||
size = m.size_bytes | ||
if cur_blocks and cur_size + size > bytes_per_split: | ||
output.append(BlockList(cur_blocks, cur_meta)) | ||
output.append( | ||
BlockList( | ||
cur_blocks, cur_meta, owned_by_consumer=self._owned_by_consumer | ||
) | ||
) | ||
cur_blocks = [] | ||
cur_meta = [] | ||
cur_size = 0 | ||
cur_blocks.append(b) | ||
cur_meta.append(m) | ||
cur_size += size | ||
if cur_blocks: | ||
output.append(BlockList(cur_blocks, cur_meta)) | ||
output.append( | ||
BlockList( | ||
cur_blocks, cur_meta, owned_by_consumer=self._owned_by_consumer | ||
) | ||
) | ||
return output | ||
|
||
def size_bytes(self) -> int: | ||
|
@@ -110,8 +133,16 @@ def divide(self, block_idx: int) -> ("BlockList", "BlockList"): | |
""" | ||
self._check_if_cleared() | ||
return ( | ||
BlockList(self._blocks[:block_idx], self._metadata[:block_idx]), | ||
BlockList(self._blocks[block_idx:], self._metadata[block_idx:]), | ||
BlockList( | ||
self._blocks[:block_idx], | ||
self._metadata[:block_idx], | ||
owned_by_consumer=self._owned_by_consumer, | ||
), | ||
BlockList( | ||
self._blocks[block_idx:], | ||
self._metadata[block_idx:], | ||
owned_by_consumer=self._owned_by_consumer, | ||
), | ||
) | ||
|
||
def get_blocks(self) -> List[ObjectRef[Block]]: | ||
|
@@ -180,4 +211,4 @@ def randomize_block_order(self, seed: Optional[int] = None) -> "BlockList": | |
random.shuffle(blocks_with_metadata) | ||
blocks, metadata = map(list, zip(*blocks_with_metadata)) | ||
|
||
return BlockList(blocks, metadata) | ||
return BlockList(blocks, metadata, owned_by_consumer=self._owned_by_consumer) |
This file contains 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 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 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it the idea that if the blocks are
owned_by_consumer
, then we can eagerly clean them up?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, once it's consumed, the blocklist can be cleared.