Skip to content

Migrate dependencies dependents sets from TaskState to Key#9042

Closed
fjetter wants to merge 5 commits intodask:mainfrom
fjetter:migrate_dependencies_dependents
Closed

Migrate dependencies dependents sets from TaskState to Key#9042
fjetter wants to merge 5 commits intodask:mainfrom
fjetter:migrate_dependencies_dependents

Conversation

@fjetter
Copy link
Member

@fjetter fjetter commented Apr 15, 2025

This is a follow up to #9036 and builds directly on top

This replaces the elements in TaskState.dependencies with the keys instead of the TaskState objects themselves. This has a few benefits

  1. It is a step towards making TaskState overhead lighter. A few months ago we were investigating GC overhead on the scheduler and the TaskState objects are adding to this in a non-trivial way. The fewer references we keep around the easier. Besides, these dependencies/dependents links are cycles that have to be followed by the GC. I don't expect this change to have a measurable impact but I believe this direction is healthy for the scheduler as a whole.
  2. More importantly, this paves the way to drop the dedicated dependencies set altogether in favor of the frozenset of the Task class (TaskState.run_spec) and reduce update_graph runtime and reduces memory overhead a little (c.f. a simple, empty set needs already 216B on py3.10, see also Reduce memory usage of scheduler process - Optimize scheduler.py::TaskState class #8331)

@github-actions
Copy link
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    27 files  ± 0      27 suites  ±0   12h 51m 31s ⏱️ + 1h 38m 14s
 4 101 tests + 1   3 951 ✅  -  33    112 💤 ±0   38 ❌ + 34 
51 413 runs  +13  48 802 ✅  - 285  2 308 💤  - 1  303 ❌ +299 

For more details on these failures, see this check.

Results for commit e72e219. ± Comparison against base commit 16aa189.

@fjetter fjetter closed this Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant