Skip to content

ci: periodic CI-duration review, and optimisations as the suite grows #1538

Closed

Description

@JarryShaw

The owner asked for a periodic review of CI time and for optimisations as the test suite grows. This is the first review, measured over the last ~55–150 Unit Tests runs (2026-10-05 to 10-09).

Where the time goes:

  • A PR becomes mergeable 14.4 min after its push (median; p90 19.6).
  • The critical path is the Python 3.14 coverage leg. It finished last in 43 of 55 PR runs, and its Run unit tests step takes 551 s against 332–372 s on 3.10–3.13 (1.57× paired, rising).
  • 71% of local unit-test time is per-class pcapkit re-imports. Tests grew 33% since test: re-import pcapkit only where isolation needs it (#1065) #1070 while test time grew ~90%.
  • The runner pool is capped at 20 concurrent jobs, and Unit Tests uses 92% of it. One PR push queues 28 jobs; one main push queues 38.
  • 18% of runner time went to superseded PR runs; about half of those were superseded by rebases.

Projections (estimates): the 20-min step cap on Plain unittest ordering (protocols (rest)) is hit in ~6–18 days, and the 3.14 leg's 45-min cap in ~10–30 days.

Starting now:

  • P3: JUnit timings and --durations from CI.
  • Periodic review: a weekly ci-timing.yml with a stdlib util/ci_timing.py. It reports job, step and test trends against thresholds anchored to today's numbers, and fails (emailing the owner) when a job's p90 reaches 50% of its cap or a projection reaches the cap within 14 days.

Needs the owner's call, with savings as re-derived by the verifiers:

# Proposal Re-derived saving Verdict
P1 Shard the coverage leg in two, plus a combine job ~1.75 min at the PR median, ~0 on push; 40% of runs slower; reverses #1063 unsound as written
P2 Stop the per-test re-imports in 7 files (13 classes; two are documented on purpose) ~22–40 s serial, ~1–3 runner-min per PR sound with changes
P4 Make the 11 ordering cells needs: test, or move them to schedule-only (b) changes what gates Pages, Conda and release; reverses #1052 H sound with changes
P6 Engines 3.15 cell on main pushes only ~3 runner-min per PR unverified
P7 Lazy TCP/UDP app-type registries (33% of import time) ~35–55 s per leg unverified
P8 Drop Engines (DPKT), which repeats test_dpkt_unit.py ~3 runner-min per run unverified

P5 (rebase only when needed) is process, not CI, and the agents have already adopted it: 27 of 43 merges in 24 h landed BEHIND through the admin bypass, so a rebase pushed only to clear BEHIND wastes runner time.

Activity

  1. added
    ciPull requests that change CI or workflow configuration (ci: subject prefix)
    wipWork in flight - a covering PR is open or an agent is actively on it
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 9, 2026
  2. moved this to Needs decision in PyPCAPKiton Oct 9, 2026
  3. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    All is good and accepted

  4. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: the owner accepted all of the remaining proposals. #1542 has already merged P3 and the weekly review. The rest, and how each will land:

    • P4, option (a): the 11 ordering cells get needs: [test] and keep running on every push, so they still gate Pages, Conda, Vendor and Release. I chose (a) because (b) would stop ordering from gating publishing, and nothing in the acceptance reverses that. Say so if you want (b).
    • P6: the Engines 3.15 cell runs on main pushes only.
    • P8: the Engines (DPKT) job is dropped.
    • P2: the per-test re-imports go, across 13 classes in 7 files, including test_logging. The deliberate purge in test_const_enum_builtin_parity.py stays.
    • P7: the TCP/UDP app-type registries load lazily. This changes library behaviour, so its PR must show every round trip still byte-exact.
    • P1: the coverage shard goes in with the verifiers' required changes: a guarded combine job, a completeness check and required-checks wiring. It is sequenced after the P4/P6/P8 PR, because both edit unit-tests.yml.

    Workers are starting now on three PRs: one for P4, P6 and P8 together, one for P2, and one for P7. Each PR re-measures its own saving rather than reusing the estimates here.

  5. removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 10, 2026
  6. moved this from Needs decision to WIP in PyPCAPKiton Oct 10, 2026
  7. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    P1 (coverage shard): new measurements say it would make CI slower. Should it still be built?

    The worker re-measured P1 before building it:

    • Critical path: over the last 15 successful PR runs, Python 3.14 finished last in only 7. I re-counted this myself. Splitting it cannot help the other 8.
    • Shard split: from run 38015219000's JUnit timings, tests/protocols + tests/project is 50.5% of the leg's test time.
    • PR runs (estimate): modelled with each run's real queue delay, the median gets 1.05 min worse. 10 of 15 runs are slower, and the gain is 0.00 min even with no queueing.
    • Push runs (estimate): all 10 are slower. The combine job queues behind the 11 ordering cells that ci: queue the ordering cells after the unit legs, run Engines 3.15 on main only, drop Engines (DPKT) (#1538) #1551 now starts at the same moment.

    Building it would also need four files beyond the brief:

    • util/ci_timing.py: COVERAGE_JOB and the junit-unit-3.14 artifact are both single-leg.
    • tests/project/test_ci_timing.py:680.
    • .github/coverage.toml.
    • docs/source/contributing/conventions/test-coverage.rst.

    Options:

    1. Drop P1. The weekly ci-timing.yml report flags if the 3.14 leg becomes the bottleneck again.
    2. Build it anyway, with those four files included.

    I recommend 1. P2, P4, P6 and P8 have merged, and P7 is in review as #1560.

  8. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 10, 2026
  9. moved this from WIP to Needs decision in PyPCAPKiton Oct 10, 2026
  10. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    take 1

  11. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: you chose option 1, so P1 (the coverage shard) is dropped. If the 3.14 leg becomes the bottleneck again, the weekly ci-timing.yml report will flag it.

    That completes this review:

    I'm closing this. The next periodic review will open its own issue when the weekly report trips.

  12. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 10, 2026
  13. moved this from Needs decision to Done in PyPCAPKiton Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ciPull requests that change CI or workflow configuration (ci: subject prefix)

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions