Skip to content

Commit 46767df

Browse files
authored
Improve Ordering reuse for groupby and join (NVIDIA#23927)
Improves cudf-polars streaming actors so existing `Ordering` metadata can be reused more effectively. - Replaces the useless `is_ordered` method with a more-general `get_ordering` method. It is really up to the consumer/actor to decide if/how an existing `Ordering` should be used to avoid unnecessary work. - Allows groupby to use `adjust_ordering` for order-partitioned input even when the input is not already strictly partitioned on the full grouping keys. For `maintain_order=True`, this avoids forcing tree reduction when ordered adjustment can preserve the necessary semantics. - Avoids unnecessary `adjust_ordering` work in ordered joins when an input side is already aligned with the target ordering. - Improves ordered-join tracing. Part of NVIDIA#22128. Authors: - Richard (Rick) Zamora (https://github.com/rjzamora) Approvers: - Tom Augspurger (https://github.com/TomAugspurger) URL: NVIDIA#23927
1 parent 4bcb104 commit 46767df

7 files changed

Lines changed: 273 additions & 74 deletions

File tree

‎python/cudf_polars/cudf_polars/streaming/actor_graph/collectives/sort.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,18 @@ def _sort_to_order_keys(ir: Sort) -> list[OrderKey]:
701701
]
702702

703703

704+
def _can_sort_chunkwise(
705+
ordering: Ordering | None, order_keys: Sequence[OrderKey]
706+
) -> bool:
707+
"""Return true when ordering avoids a global sort."""
708+
if ordering is None:
709+
return False
710+
keys = tuple(ordering.keys)
711+
return keys == tuple(order_keys[: len(keys)]) and (
712+
len(keys) == len(order_keys) or ordering.strict_boundaries
713+
)
714+
715+
704716
def _build_order_scheme(
705717
context: Context,
706718
order_keys: list[OrderKey],
@@ -832,10 +844,10 @@ async def sort_actor(
832844
partitioning = NormalizedPartitioning.from_keys(
833845
metadata_in.partitioning, comm.nranks, keys=order_keys
834846
)
835-
if partitioning.is_ordered(
836-
order_keys,
837-
level="local" if metadata_in.duplicated else "flat",
838-
):
847+
ordering = partitioning.get_ordering(
848+
level="local" if metadata_in.duplicated else "flat"
849+
)
850+
if _can_sort_chunkwise(ordering, order_keys):
839851
if tracer is not None:
840852
tracer.decision = "already_sorted"
841853
await chunkwise_evaluate(

‎python/cudf_polars/cudf_polars/streaming/actor_graph/groupby.py‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,13 @@ async def groupby_actor(
891891
fully_partitioned = partitioning.is_strictly_partitioned(
892892
level=partitioning_level,
893893
)
894+
input_ordering = None if metadata_in.duplicated else partitioning.get_ordering()
895+
can_adjust_preserving_order = (
896+
isinstance(ir, GroupBy)
897+
and preserves_output_order
898+
and input_ordering is not None
899+
and not _has_stable_sorted_agg(ir.agg_requests)
900+
)
894901
fallback_case = (
895902
# NOTE: This criteria means that we fell back
896903
# to one partition at lowering time.
@@ -936,7 +943,7 @@ async def groupby_actor(
936943
ir_context,
937944
ch_in,
938945
target_partition_size,
939-
allow_early_exit=not maintain_order,
946+
allow_early_exit=not maintain_order or can_adjust_preserving_order,
940947
)
941948

942949
skip_global_comm = metadata_in.duplicated or isinstance(
@@ -952,7 +959,7 @@ async def groupby_actor(
952959
collective_ids,
953960
target_partition_size,
954961
skip_global_comm,
955-
maintain_order,
962+
maintain_order and not can_adjust_preserving_order,
956963
tracer,
957964
)
958965

@@ -969,11 +976,7 @@ async def groupby_actor(
969976
aggregated=aggregated,
970977
tracer=tracer,
971978
)
972-
elif not metadata_in.duplicated and partitioning.is_ordered(
973-
group_keys,
974-
level="flat",
975-
):
976-
assert isinstance(partitioning.inter_rank_scheme, OrderScheme)
979+
elif input_ordering is not None:
977980
await _ordered_adjust_reduce(
978981
context,
979982
comm,
@@ -986,7 +989,7 @@ async def groupby_actor(
986989
target_partition_size,
987990
aggregated=aggregated,
988991
input_drained=input_drained,
989-
input_ordering=partitioning.inter_rank_scheme.orderings[0],
992+
input_ordering=input_ordering,
990993
preserves_output_order=preserves_output_order,
991994
tracer=tracer,
992995
)

‎python/cudf_polars/cudf_polars/streaming/actor_graph/join.py‎

Lines changed: 79 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,12 @@ class OrderedJoinStrategy:
139139
JoinStrategy: TypeAlias = (
140140
BroadcastJoinStrategy | ShuffleJoinStrategy | OrderedJoinStrategy
141141
)
142+
OrderedJoinDecision: TypeAlias = Literal[
143+
"ordered_aligned",
144+
"ordered_adjust_left",
145+
"ordered_adjust_right",
146+
"ordered_adjust_both",
147+
]
142148

143149

144150
@define_actor()
@@ -538,6 +544,17 @@ def _make_ordered_strategy(
538544
if not _ordering_prefix_matches(right_ordering, reference, right_key_indices):
539545
return None
540546

547+
left_output_ordering = _update_ordering_indices(reference, left_key_indices)
548+
output_ordering = _update_ordering_indices(reference, output_key_indices)
549+
if not (
550+
left_ordering.locally_ordered
551+
and join_preserves_side_order(ir.options[5], "left")
552+
):
553+
left_output_ordering = left_output_ordering.with_locally_ordered(
554+
locally_ordered=False
555+
)
556+
output_ordering = output_ordering.with_locally_ordered(locally_ordered=False)
557+
541558
return OrderedJoinStrategy(
542559
output_indices=output_key_indices,
543560
left_indices=left_key_indices,
@@ -546,9 +563,11 @@ def _make_ordered_strategy(
546563
right_keys=right_keys[:reference_key_count],
547564
left_input_ordering=left_ordering,
548565
right_input_ordering=right_ordering,
549-
left_output_ordering=_update_ordering_indices(reference, left_key_indices),
550-
right_output_ordering=_update_ordering_indices(reference, right_key_indices),
551-
output_ordering=_update_ordering_indices(reference, output_key_indices),
566+
left_output_ordering=left_output_ordering,
567+
right_output_ordering=_update_ordering_indices(
568+
reference, right_key_indices
569+
).with_locally_ordered(locally_ordered=False),
570+
output_ordering=output_ordering,
552571
)
553572

554573

@@ -744,6 +763,19 @@ def _local_count_for_ordering(comm: Communicator, ordering: Ordering) -> int:
744763
return stop - start
745764

746765

766+
def _ordered_join_decision(
767+
*, left_aligned: bool, right_aligned: bool
768+
) -> OrderedJoinDecision:
769+
"""Return the trace decision for ordered join-side alignment."""
770+
if left_aligned and right_aligned:
771+
return "ordered_aligned"
772+
if left_aligned:
773+
return "ordered_adjust_right"
774+
if right_aligned:
775+
return "ordered_adjust_left"
776+
return "ordered_adjust_both"
777+
778+
747779
async def _adjust_ordered_join_side(
748780
context: Context,
749781
comm: Communicator,
@@ -754,21 +786,30 @@ async def _adjust_ordered_join_side(
754786
input_ordering: Ordering,
755787
output_ordering: Ordering,
756788
*,
789+
already_aligned: bool,
757790
collective_id: int,
758791
) -> None:
759792
"""Send metadata, then align one join side to output_ordering."""
760-
await send_metadata(
761-
ch_out,
762-
context,
763-
ChannelMetadata(
764-
local_count=_local_count_for_ordering(comm, output_ordering),
765-
partitioning=Partitioning(
766-
OrderScheme([output_ordering]),
767-
local="inherit",
768-
),
769-
duplicated=False,
793+
output_metadata = ChannelMetadata(
794+
local_count=_local_count_for_ordering(comm, output_ordering),
795+
partitioning=Partitioning(
796+
OrderScheme([output_ordering]),
797+
local="inherit",
770798
),
799+
duplicated=False,
771800
)
801+
if already_aligned:
802+
await replay_buffered_channel(
803+
context,
804+
ch_out,
805+
ch_in,
806+
(),
807+
output_metadata,
808+
trace_ir=schema_ir,
809+
)
810+
return
811+
812+
await send_metadata(ch_out, context, output_metadata)
772813
await adjust_ordering(
773814
context,
774815
comm,
@@ -796,6 +837,12 @@ async def _ordered_join(
796837
tracer: ActorTracer | None,
797838
) -> None:
798839
"""Align ordered inputs to common boundaries, then join partition-wise."""
840+
left_boundaries_aligned = strategy.left_input_ordering.boundaries_aligned_with(
841+
strategy.left_output_ordering, context.br()
842+
)
843+
right_boundaries_aligned = strategy.right_input_ordering.boundaries_aligned_with(
844+
strategy.right_output_ordering, context.br()
845+
)
799846
metadata_out = ChannelMetadata(
800847
local_count=_local_count_for_ordering(comm, strategy.output_ordering),
801848
partitioning=Partitioning(
@@ -806,10 +853,26 @@ async def _ordered_join(
806853
)
807854
await send_metadata(ch_out, context, metadata_out)
808855

809-
await gather_in_task_group(
856+
left_metadata, right_metadata = await gather_in_task_group(
810857
recv_metadata(ch_left, context),
811858
recv_metadata(ch_right, context),
812859
)
860+
left_target_count = _local_count_for_ordering(comm, strategy.left_output_ordering)
861+
right_target_count = _local_count_for_ordering(comm, strategy.right_output_ordering)
862+
left_aligned = (
863+
left_boundaries_aligned
864+
and not left_metadata.duplicated
865+
and left_metadata.local_count == left_target_count
866+
)
867+
right_aligned = (
868+
right_boundaries_aligned
869+
and not right_metadata.duplicated
870+
and right_metadata.local_count == right_target_count
871+
)
872+
if tracer is not None:
873+
tracer.decision = _ordered_join_decision(
874+
left_aligned=left_aligned, right_aligned=right_aligned
875+
)
813876
ch_left_adjusted = context.create_channel()
814877
ch_right_adjusted = context.create_channel()
815878
async with shutdown_on_error(
@@ -829,6 +892,7 @@ async def _ordered_join(
829892
ch_left,
830893
strategy.left_input_ordering,
831894
strategy.left_output_ordering,
895+
already_aligned=left_aligned,
832896
collective_id=collective_ids.pop(0),
833897
),
834898
_adjust_ordered_join_side(
@@ -840,6 +904,7 @@ async def _ordered_join(
840904
ch_right,
841905
strategy.right_input_ordering,
842906
strategy.right_output_ordering,
907+
already_aligned=right_aligned,
843908
collective_id=collective_ids.pop(0),
844909
),
845910
_join_chunks(

‎python/cudf_polars/cudf_polars/streaming/actor_graph/utils.py‎

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
Callable,
5555
Coroutine,
5656
Generator,
57+
Iterable,
5758
Iterator,
5859
Sequence,
5960
)
@@ -1278,7 +1279,7 @@ async def replay_buffered_channel(
12781279
context: Context,
12791280
ch_out: Channel[TableChunk],
12801281
ch_in: Channel[TableChunk],
1281-
buffered_chunks: ChunkStore,
1282+
buffered_chunks: Iterable[Message],
12821283
metadata: ChannelMetadata,
12831284
*,
12841285
trace_ir: IR,
@@ -1295,7 +1296,7 @@ async def replay_buffered_channel(
12951296
ch_in
12961297
The buffered input channel.
12971298
buffered_chunks
1298-
The buffered chunks to yield first.
1299+
Buffered messages to yield first. May be empty.
12991300
metadata
13001301
The metadata to send to the output channel.
13011302
trace_ir
@@ -1358,29 +1359,6 @@ def _scheme_is_strict(scheme: PartitioningScheme) -> bool:
13581359
return scheme.orderings[0].strict_boundaries
13591360
return True
13601361

1361-
@staticmethod
1362-
def _ordering_covers_keys(
1363-
ordering: Ordering,
1364-
order_keys: Sequence[int | OrderKey],
1365-
) -> bool:
1366-
"""True when an ordering covers the requested sort keys."""
1367-
ordering_keys = ordering.keys
1368-
if len(ordering.keys) < len(order_keys):
1369-
# If we are only sorted on a subset of the keys, we need strict
1370-
# boundaries to know later keys cannot interleave across chunks.
1371-
if not ordering.strict_boundaries:
1372-
return False
1373-
order_keys = order_keys[: len(ordering.keys)]
1374-
else:
1375-
ordering_keys = ordering.keys[: len(order_keys)]
1376-
for current, target in zip(ordering_keys, order_keys, strict=True):
1377-
if isinstance(target, OrderKey):
1378-
if current != target:
1379-
return False
1380-
elif current.column_index != target:
1381-
return False
1382-
return True
1383-
13841362
def is_strictly_partitioned(
13851363
self,
13861364
*,
@@ -1389,17 +1367,10 @@ def is_strictly_partitioned(
13891367
"""True if data is strictly partitioned at the requested level."""
13901368
return self._scheme_is_strict(self._scheme_for_level(level))
13911369

1392-
def is_ordered(
1393-
self,
1394-
order_keys: Sequence[int | OrderKey],
1395-
*,
1396-
level: PartitioningLevel = "flat",
1397-
) -> bool:
1398-
"""True if the selected ordering covers order_keys."""
1370+
def get_ordering(self, *, level: PartitioningLevel = "flat") -> Ordering | None:
1371+
"""Return the normalized ordering for the requested partitioning level."""
13991372
scheme = self._scheme_for_level(level)
1400-
if not isinstance(scheme, OrderScheme):
1401-
return False
1402-
return self._ordering_covers_keys(scheme.orderings[0], order_keys)
1373+
return scheme.orderings[0] if isinstance(scheme, OrderScheme) else None
14031374

14041375
def is_aligned_with(
14051376
self, other: NormalizedPartitioning, br: BufferResource

0 commit comments

Comments
 (0)