Repository navigation
Speed up TaskGroup.topological_sort with int-indexed projected sweep - #67288
Merged
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Replaces the modified-Kahn body of
TaskGroup.topological_sort(and the mirror inSerializedTaskGroup) with a projected sweep:upstream_task_idsis projected onto sibling-level integer indices once up front (_project_child_deps)._sweep_projection) then runs against the projection with abytearray-backed emission flag - per-edge work happens once per sort instead of once per outer-loop pass.Emission order is identical to the previous implementation. The existing order-sensitive tests (
test_topological_sort1/2,test_topological_nested_groups,test_topological_group_dep) cover the contract.Benchmark
Min of five runs, N=2000 children, ms per call. Source: https://gist.github.com/shahar1/9c61dc9f34f7e77cd29cfb9d67af7ceb
The 7.8x on rev-chain comes from the projection precompute alone (one projection vs N projections); algorithmic complexity is unchanged and remains O(N²) on that shape until the follow-up PR.
The nested row times a recursive walk across all 39 nested groups (depth=3, ~222 tasks each); the root-only timing was dominated by setup overhead and was previously labelled
~noisein this table.Test plan
task-sdk/tests/task_sdk/definitions/test_taskgroup.py+test_dag.py— 145 passed (18 new shape-correctness cases)airflow-core/tests/unit/utils/test_task_group.py— 20 passed (1 new serialization round-trip case)airflow-core/tests/unit/serialization/(task-group / topological filter) — 4 passedprek run mypy-task-sdk— passedprek run mypy-airflow-core— passedprek run --from-ref upstream/main --stage pre-commit— passedWas generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines