dabla opened a new pull request, #74562: URL: https://github.com/apache/airflow/pull/74562
> **Review only this change:** [the PR's own commit](https://github.com/dabla/airflow/commit/9fd489f3485acdbde4fcb431c10a3b3212331381) · [diff against #62922's branch](https://github.com/dabla/airflow/compare/feature/dynamic-task-iteration...feature/micro-benchmarks). Everything else in this PR's commit list and file list is #62922, which this PR is stacked on. **Draft, opened to start a discussion on the dev list.** Stacked on #62922 (Task Iteration) only because the measurements below come from a fix there; the benchmarks themselves do not depend on it and can be rebased on `main` on their own. related: #62922 ## Why Airflow has no automated check on the performance of its hot paths. A change that makes them slower is only noticed when someone happens to measure it by hand, often after a release: #71721 reports structured task logging in 3.2.0 multiplying high-volume task runtime by 2.64. A fix on #62922 shows how much is at stake in a single method. `CommsDecoder.asend`, which every async SDK call of a task goes through (a Variable, a Connection, an XCom, a state store key), made two hops to a worker thread per request; it now makes one. Measured with the benchmarks of this PR, old `comms.py` against new, on the same machine, the middle value of three runs (pytest-codspeed, walltime mode, best time per round): | Benchmark | Before | After | Change | |---|---|---|---| | `send`: one sync round trip (code unchanged, the control) | 11.5 µs | 11.4 µs | none | | `asend`: one async round trip | 108.7 µs | 58.0 µs | **−47%** | | `asend`: 50 concurrent coroutines on one loop | 18.6 ms | 11.7 ms | **−37%** | An iterated async task makes such calls for every item, so the saving repeats thousands of times per task (the supervisor's and API server's own share of each call is not part of this measurement). The opposite change, an extra thread hop added in passing, would cost as much, and nothing in CI would notice today. A separate ad hoc measurement (4000 `GetVariable` round trips, median of 7 interleaved rounds) agrees: 525 µs to 318 µs per request. ## What - `task-sdk/benchmarks/`: a new directory next to `tests/`, for benchmarks only. The first module, `execution_time/test_comms_benchmark.py`, measures one sync round trip, one async round trip and fifty concurrent async round trips against a supervisor thread on a socket pair. The task SDK's pytest configuration only collects `tests/`, so the benchmarks never run with the unit tests. - A `benchmarks` dependency group in `task-sdk/pyproject.toml` with [pytest-codspeed](https://github.com/CodSpeedHQ/pytest-codspeed) (MIT). Benchmarks are plain pytest functions using its `benchmark` fixture. Locally they run with `uv run --project task-sdk --group benchmarks pytest task-sdk/benchmarks --codspeed`. - `.github/workflows/benchmarks.yml`: runs them on pushes to `main` and on PRs touching `task-sdk/src/airflow/sdk/execution_time/` or the benchmarks, through `CodSpeedHQ/action` in simulation mode. That mode counts CPU instructions instead of measuring time, so the results do not depend on how busy the shared runner is, and CodSpeed comments each PR with the change against `main`. The action is already on the ASF allow-list (`CodSpeedHQ/action` v5.2.1, pinned by SHA in [apache/infrastructure-actions](https://github.com/apache/infrastructure-actions/blob/main/actions.yml)). It authenticates through OIDC, so no secret is needed for a public repository, but someone with admin rights has to enable the CodSpeed app for `apache/airflow` before the workflow can report anything. ## Questions for the dev list 1. **Do we want micro-benchmarks at all?** [AIP-59](https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-59+Performance+tests+framework) (Performance tests framework, accepted, still in progress) measures the footprint of whole components: scheduler, workers, triggerer and the database. Micro-benchmarks would be the layer below it, for single methods on hot paths, catching a regression in the PR that introduces it. Is that an extension of AIP-59, or does it need an AIP of its own? 2. **Where should they live and how should they be named?** This draft uses `<distribution>/benchmarks/`, next to `tests/`, with `test_*_benchmark.py` files so the existing pytest configuration collects them without changes. A `bench_*.py` convention would need its own `python_files` setting. 3. **Which tool?** pytest-codspeed with the hosted service compares against `main` on each PR, with results that do not depend on runner load. pytest-benchmark is purely local and compares against a saved baseline with a threshold, but wall-time thresholds on shared CI runners are noisy, so they either fail at random or are set too loose to catch a drift. 4. **What should a regression do?** Report only (a PR comment), or fail the check above some threshold? 5. **Which paths come first?** Candidates beyond the comms: XCom serialisation, template rendering, the DAG processor's parsing, and the scheduler's critical section queries. No newsfragment: nothing here is user-facing. --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes (please specify the tool below) Generated-by: Claude Code (Opus 5.5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions) --- * Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged. * For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
