Skip to content

Conversation

@hekaisheng
Copy link
Contributor

@hekaisheng hekaisheng commented Apr 18, 2022

What do these changes do?

When trigger iterative tiling, subtask's graph may has fetch operand, and we treat them as initial nodes and assign them in GraphAnalyzer, it will result in losing locality and bad performance. This PR try to solve this issue by ignoring fetch chunks when assign chunk graph.

Related issue number

Fixes #xxxx

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

"""Get available band slots."""

@abstractmethod
async def get_chunk_bands(self, chunk_keys: List[str]) -> Dict[str, BandType]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

About if it is reasonable to add this API to execution, what's your opinion? @fyrestone

Copy link
Contributor

Choose a reason for hiding this comment

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

The API is a bit wired for execution. From the title of the PR, I think may be the changes are related to graph construction not the execution?

Copy link
Contributor

@fyrestone fyrestone Apr 19, 2022

Choose a reason for hiding this comment

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

@wjsi wjsi force-pushed the enh/assign-fetch branch from f4f8331 to 1deb2c8 Compare April 18, 2022 14:59
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM

@wjsi wjsi merged commit 1c4fbf7 into mars-project:master Apr 19, 2022
@qinxuye qinxuye deleted the enh/assign-fetch branch April 19, 2022 11:24
qinxuye pushed a commit to qinxuye/mars that referenced this pull request Apr 21, 2022
qinxuye pushed a commit to qinxuye/mars that referenced this pull request Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants