-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add IR-fusion optimization pass to streaming cudf-polars executor #18797
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
Changes from 19 commits
247eaaf
2c882d7
7c986bc
65bff68
ecf0db5
21ec311
3da460c
6c57bfa
8695863
08e8205
ddab700
502ac9f
ccb909f
fddf093
24e4ce5
6db02ec
768e56c
a857850
981a25b
c129bc3
f9056ed
0e9bd96
d03fc03
5e014db
c4751d2
deb549b
558aad8
7cf61ef
d07c350
00e8098
e34530b
e6636b3
8c6efcf
9daf47b
4fa356c
b5384f7
15f2332
772f584
7e26603
b68a241
d21da69
c5807e7
924b4e5
517c081
4da62d4
28a6f8b
1c6b3a3
2ae7e08
5ad4b1a
f60282f
19cbf46
523f750
519ba25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2024-2025, NVIDIA CORPORATION & AFFILIATES. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| """IR node fusion.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import operator | ||
| from collections import defaultdict | ||
| from functools import reduce | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from cudf_polars.dsl.ir import ( | ||
| IR, | ||
| Cache, | ||
| Filter, | ||
| GroupBy, | ||
| HStack, | ||
| MapFunction, | ||
| Projection, | ||
| Select, | ||
| Sort, | ||
| Union, | ||
| ) | ||
| from cudf_polars.dsl.traversal import CachingVisitor, traversal | ||
| from cudf_polars.experimental.dispatch import generate_ir_tasks | ||
| from cudf_polars.experimental.io import Scan, SplitScan | ||
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Callable, MutableMapping, Sequence | ||
| from typing import Any | ||
|
|
||
| from cudf_polars.containers import DataFrame | ||
| from cudf_polars.experimental.base import PartitionInfo | ||
| from cudf_polars.experimental.dispatch import LowerIRTransformer | ||
| from cudf_polars.typing import Schema | ||
| from cudf_polars.utils.config import ConfigOptions | ||
|
|
||
|
|
||
| class Fused(IR): | ||
| """Fused IR node.""" | ||
|
|
||
| __slots__ = ("fused_io", "subnodes") | ||
| _non_child = ("schema", "subnodes", "fused_io") | ||
| subnodes: tuple[IR, ...] | ||
| """Fused sub-nodes.""" | ||
| fused_io: Union | None | ||
| """Fused IO node.""" | ||
|
|
||
| def __init__( | ||
| self, | ||
| schema: Schema, | ||
| subnodes: tuple[IR, ...], | ||
| fused_io: Union | None, | ||
| *children: IR, | ||
| ): | ||
| self.schema = schema | ||
| self.subnodes = subnodes | ||
| self.fused_io = fused_io | ||
| self._non_child_args = ( | ||
| [node.do_evaluate for node in subnodes], | ||
| [tuple(node._non_child_args) for node in self.subnodes], | ||
| [], | ||
| [], | ||
|
Member
Author
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. Note: I needed to use lists rather than tuples or callable objects, because we run into issues when the scheduler or rapimdspf-spilling things these are tasks.
Contributor
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. 😢 |
||
| ) | ||
| self.children = children | ||
|
|
||
| @classmethod | ||
| def do_evaluate( | ||
| cls, | ||
| funcs: Sequence[Callable], | ||
| subargs: Sequence[Sequence[Any]], | ||
| io_funcs: Sequence[Callable], | ||
| io_args: Sequence[Any], | ||
| *children: DataFrame, | ||
| ) -> DataFrame: | ||
| """Evaluate and return a dataframe.""" | ||
| if io_funcs: | ||
| children = (io_funcs[0](*io_args),) | ||
|
Contributor
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. For IO, we know that the passed in |
||
| for func, args in zip(funcs, subargs, strict=False): | ||
|
rjzamora marked this conversation as resolved.
Outdated
|
||
| children = (func(*args, *children),) | ||
| return children[0] | ||
|
|
||
|
|
||
| def _fuse_ir_node( | ||
| ir: IR, rec: LowerIRTransformer | ||
| ) -> tuple[IR, MutableMapping[IR, PartitionInfo]]: | ||
| partition_info: MutableMapping[IR, PartitionInfo] = {} | ||
| children = () | ||
| if ir.children: | ||
| children, _partition_info = zip(*(rec(c) for c in ir.children), strict=True) | ||
| partition_info = reduce(operator.or_, _partition_info) | ||
|
|
||
| new_node: IR | ||
| if ir in rec.state["fusable"]: | ||
| if isinstance(ir, Union): | ||
| new_node = Fused(ir.schema, (), ir) | ||
|
Contributor
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. Because we've already marked this node as fusible, we know it must be an IO node?
Member
Author
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. Yeah, I refactored things a bit so that the |
||
| elif len(children) == 1 and isinstance(children[0], Fused): | ||
| (child,) = children | ||
| grandchildren = child.children | ||
| new_node = Fused( | ||
| ir.schema, (*child.subnodes, ir), child.fused_io, *grandchildren | ||
| ) | ||
|
Contributor
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. So this is kind of "tricking" the task generation routine into doing partition-wise operations without generating new tasks/intermediates.
Contributor
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. Trying to understand how this works. I think this is trying to fuse things like: So we only generate a "task" for evaluating But I think this rewrites to: But it seems like the evaluation rule would produce
Member
Author
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. We are actually targeting simple sequential IR-node strings. |
||
| else: | ||
| new_node = Fused(ir.schema, (ir,), None, *children) | ||
| else: | ||
| new_node = ir.reconstruct(children) | ||
|
|
||
| partition_info[new_node] = rec.state["partition_info"][ir] | ||
| return new_node, partition_info | ||
|
|
||
|
|
||
| def _fusable_ir_type(ir: IR) -> bool: | ||
|
TomAugspurger marked this conversation as resolved.
Outdated
|
||
| return ( | ||
| # Basic fusion | ||
| isinstance( | ||
| ir, (Cache, Filter, GroupBy, HStack, MapFunction, Projection, Select, Sort) | ||
| ) | ||
| or ( | ||
| # IO Fusion | ||
| isinstance(ir, Union) | ||
| and all(isinstance(n, (Scan, SplitScan)) for n in ir.children) | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| @generate_ir_tasks.register(Fused) | ||
| def _( | ||
| ir: Fused, partition_info: MutableMapping[IR, PartitionInfo] | ||
| ) -> MutableMapping[Any, Any]: | ||
| from cudf_polars.experimental.parallel import _default_generate_ir_tasks | ||
|
|
||
| if ir.fused_io is None: | ||
| return _default_generate_ir_tasks(ir, partition_info) | ||
|
|
||
| assert isinstance(ir.fused_io, Union) | ||
|
rjzamora marked this conversation as resolved.
Outdated
|
||
| graph: MutableMapping[Any, Any] = {} | ||
| for i, key in enumerate(partition_info[ir].keys(ir)): | ||
| io = ir.fused_io.children[i] | ||
|
TomAugspurger marked this conversation as resolved.
|
||
| assert isinstance(io, (Scan, SplitScan)) | ||
| graph[key] = ( | ||
| ir.do_evaluate, | ||
| ir._non_child_args[0], | ||
| ir._non_child_args[1], | ||
| [io.do_evaluate], | ||
| list(io._non_child_args), | ||
| ) | ||
| return graph | ||
|
|
||
|
|
||
| def fuse_ir_graph( | ||
| ir: IR, | ||
| partition_info: MutableMapping[IR, PartitionInfo], | ||
| config_options: ConfigOptions, | ||
| ) -> tuple[IR, MutableMapping[IR, PartitionInfo]]: | ||
| """ | ||
| Rewrite an IR graph with fused nodes. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| ir | ||
| Root of the graph to rewrite. | ||
|
rjzamora marked this conversation as resolved.
Outdated
|
||
| partition_info | ||
| Initial partitioning information. | ||
| config_options | ||
| GPUEngine configuration options. | ||
|
|
||
| Returns | ||
| ------- | ||
| new_ir, partition_info | ||
| The rewritten graph, and a mapping from unique nodes | ||
| in the new graph to associated partitioning information. | ||
| """ | ||
| assert config_options.executor.name == "streaming", ( | ||
| "'in-memory' executor not supported in 'fuse_ir_graph'" | ||
| ) | ||
| if not config_options.executor.task_fusion: | ||
|
rjzamora marked this conversation as resolved.
Outdated
|
||
| return ir, partition_info | ||
|
|
||
| parents: defaultdict[IR, int] = defaultdict(int) | ||
| if _fusable_ir_type(ir): | ||
| parents[ir] = 1 | ||
| for node in traversal([ir]): | ||
| for child in node.children: | ||
| if _fusable_ir_type(child): | ||
| # Basic fusion | ||
| parents[child] += 1 | ||
| fusable = {node for node, count in parents.items() if count == 1} | ||
| state = {"fusable": fusable, "partition_info": partition_info} | ||
| mapper = CachingVisitor(_fuse_ir_node, state=state) | ||
| return mapper(ir) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,6 @@ | |
| import dataclasses | ||
| import enum | ||
| import math | ||
| import random | ||
| from enum import IntEnum | ||
| from typing import TYPE_CHECKING, Any, TypeVar | ||
|
|
||
|
|
@@ -263,9 +262,10 @@ def _sample_pq_statistics(ir: Scan) -> dict[str, np.floating[T]]: | |
| import numpy as np | ||
|
|
||
| # Use average total_uncompressed_size of three files | ||
| n_sample = min(3, len(ir.paths)) | ||
| n_sample = 5 # TODO: Make this configurable | ||
| stride = max(1, int(len(ir.paths) / n_sample)) | ||
| metadata = plc.io.parquet_metadata.read_parquet_metadata( | ||
| plc.io.SourceInfo(random.sample(ir.paths, n_sample)) | ||
| plc.io.SourceInfo(ir.paths[: stride * n_sample : stride]) | ||
|
Member
Author
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. This "fix" is not related to the rest of the PR, but it made it easier to do A-B testing with the new fusion optimization. |
||
| ) | ||
| column_sizes = {} | ||
| rowgroup_offsets_per_file = np.insert( | ||
|
|
||
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.
question: It seems like
fused_iois never used in thedo_evaluate. How does this work? Ah, it's because the task generation does the right thing.Does that suggest we should have separate
FusedandFusedIOnodes? So we don't have this dichotomy?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.
FYI: I refactored things with this goal in mind, and cleaned things up a bit in the process.