Skip to content

fix(pycg)!: decide shard outcomes by convergence, not wall clock - #147

Closed
rahlk wants to merge 4 commits into
mainfrom
fix/issue-145-deterministic-sharding
Closed

rahlk wants to merge 4 commits into
mainfrom
fix/issue-145-deterministic-sharding

Conversation

@rahlk

@rahlk rahlk commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Closes #145.

Problem

Sharded PyCG decided which shards to keep by wall-clock timeout, and a
timed-out shard contributed zero edges. Which shards run slow depends on
machine load and Ray scheduling, so byte-identical invocations produced
different call graphs — measured on a 2,364-file dependency-closed Odoo subset
with --no-venv --pycg-shard --ray:

run edges jedi pycg jedi,pycg externals
-a 2 48,595 26,309 18,962 3,324 4,492
-a 2 (repeat) 43,431 27,720 13,751 1,960 3,340
-a 4 40,224 28,046 10,529 1,649 2,881

An 11% spread overall; PyCG's own contribution swings 44%.

The issue as filed asked for the incompleteness to be reported in the payload.
That was rejected: a self-describing non-deterministic output is still
non-deterministic. This removes the load-dependence at its source.

The fix

PyCG's loop is
while (max_iter < 0 or iter_cnt < max_iter) and not has_converged(), so shard
outcomes can be decided on convergence instead of the clock (55965a4):

  • Shard outcomes now come from PyCG's own convergence — a function of the
    input alone, not of machine load.
  • SIGALRM helper, Ray deadlines and ray.cancel removed from all four
    collection paths.
  • A runaway — fixpoint stopped at --pycg-max-iter instead of converging, or
    raised — is re-partitioned at a tighter file budget, as before; a shard that
    cannot be split further keeps the edges it derived instead of contributing
    zero. A capped fixpoint is a sound under-approximation, so those edges are
    real.
  • _PYCG_DECOMP_FLOOR 10 → 1: a floor of 10 left small runaways unsplittable.

Correction to a claim in the first commit message. It argues the clock
bound was redundant because --pycg-max-iter already guarantees
termination. That is wrong; the message is left as-is for history and
corrected in 7a71a6d. The cap bounds fixpoint passes, and PyCG consults it
only between passes, so a single expensive pass escapes it — one 100-file
shard has run for over an hour inside its pass budget without completing, even
seed-pinned. Removing the timeout removes the only wall-clock bound that
existed. It had to go — it decided output by machine load — but nothing
replaces it yet: #148 tracks a deterministic fallback that routes
predicted-expensive shards to Jedi-only coverage, with the degradation visible
in prov.

A reverted experiment sits in this branch's history. 98ecf05 switched
capped shards to be kept whole instead of re-split, on a measurement showing
splitting "destroyed edges". That comparison was confounded — the whole-shard
run completed three fixpoint passes while the halved run was cut off by the
measurement harness after one, so it compared different analysis depths — and
the entire series ran with an unpinned hash seed (see Measurement caveat).
A depth-matched control favored re-splitting. cfd4d75 restores the
reviewed behaviour: capped → re-split, irreducible → keep partial edges.

Reading convergence correctly

_analyze_with_convergence() is not a post-hoc has_converged() call. Two
traps, both covered by tests:

  • Asking after analyze() returns is wrong — analyze runs a
    CallGraphProcessor pass past the loop, so a post-hoc call compares state
    that pass has already moved and reports divergence for a converged shard.
  • The loop's last recorded value is also wrong — when the cap stops the
    loop the and short-circuits, leaving the False that admitted the final
    pass, mislabelling a shard that converged on exactly pass max_iter.

So the cap is raised by one and the loop cut off from inside the check: one
extra question, not an extra pass.

Breaking change

--pycg-shard-timeout is removed. It is the defect, and after this change
it controls nothing. --pycg-max-iter remains the knob trading recall against
runtime — but it caps fixpoint passes, not their duration, so no setting of
it bounds wall-clock time, and --pycg-max-iter -1 (run to convergence) can
run indefinitely.

Scope — what this does NOT claim

This removes the load-dependent shard-dropping mechanism, the 11% effect #145
was filed for. Output is not yet byte-identical across runs. A separate,
~100x smaller source remains in Jedi's overload resolution for open():
f.read() resolves to _TextIOBase.read or _BufferedIOBase.read depending
on the run, accounting for 0.1–0.3% of edges on the Flask fixture. Filed as
#146 with a full four-way matrix (sequential/Ray × max_iter 3/50)
showing it is independent of sharding, Ray, and the fixpoint cap.

I was not able to obtain a completed Odoo run demonstrating the fix
end-to-end at scale — every attempt was killed, capped, or interrupted, and a
seed-pinned attempt at one shard did not finish within an hour (the liveness
gap #148 exists for). The fix rests on the mechanism being structurally
removed plus tests asserting it, not on a before/after Odoo comparison.

Measurement caveat

Two things poisoned nearly every standalone measurement taken during this
work:

  • Unpinned hash seed. The CLI pins PYTHONHASHSEED=0 by re-exec
    (_pin_hash_seed), but only when argv0 is the CLI. Standalone probes
    import the library, so they ran with a random seed — and PyCG iterates
    hash-ordered containers, so edge counts and per-pass cost both swing with
    the seed: the same shard at the same settings produced 115,904 / 110,490 /
    73,957 edges across probe runs, completing in minutes on one seed and
    running 80+ minutes on another. Library consumers get no pin either — any
    import-based harness must set the seed itself before its numbers mean
    anything.
  • Machine state. Two dramatic wall-clock anomalies were the laptop lid
    being closed mid-run.

Earlier drafts of this PR quoted figures from that series; all are removed.
What remains — the #145 spread table (real, seed-pinned CLI runs), the
structural removal of the timeout, and the tests — is deterministic and
verifiable.

Tests

test/test_pycg_shard_determinism.py (16 tests), including:

  • a whole-module sweep asserting no SIGALRM, signal.alarm, ray.cancel or
    shard_timeout remains reachable — the timeout was reachable from four
    collection paths, so a check scoped to one function would pass while another
    still dropped shards by the clock;
  • unit tests for _analyze_with_convergence against a fake with PyCG's exact
    loop shape: converged, capped, the exactly-at-cap boundary, the
    post-loop-pass trap, unbounded max_iter=-1, degenerate max_iter=0, cap
    restoration, and restoration when analyze() raises;
  • both the sequential and Ray paths checked for runaway classification and for
    the irreducible-shard-keeps-partial-edges behaviour — a change applied to
    one runner only would silently leave --ray on the old behaviour.

Full suite: 305 passed, 6 skipped.

Docs

rahlk added 3 commits August 21, 2026 12:46
Sharded PyCG dropped shards that exceeded a wall-clock timeout, and a
dropped shard contributed zero edges. Which shards ran slow depends on
machine load and Ray scheduling, so byte-identical invocations produced
different call graphs: three runs over one 2,364-file fixture gave 48,595
/ 43,431 / 40,224 edges, an 11% spread with PyCG's own contribution
swinging 44%.

The clock bound was redundant. PyCG's fixpoint loop is

    while (max_iter < 0 or iter_cnt < max_iter) and not has_converged():

so --pycg-max-iter (default 50) already guarantees termination. Bounding
it a second time by the clock added nothing but the load-dependence.

A shard is now a runaway when its fixpoint stopped at max_iter instead of
converging -- a function of the input alone. Adaptive decomposition is
unchanged, since that is what recovers recall. A runaway that cannot be
split further now keeps the edges it did derive: a capped fixpoint is a
sound under-approximation, so discarding them was pure recall loss on top
of the non-determinism. _PYCG_DECOMP_FLOOR drops 10 -> 1, because a floor
of 10 left small runaways unsplittable and forced them down that path.

Reading convergence needs care in two places, both covered by tests.
Asking cg.has_converged() after analyze() returns is wrong: analyze runs a
CallGraphProcessor pass past the loop, so a post-hoc call compares state
that pass has already moved. The loop's last recorded value is also wrong:
when the cap stops the loop the `and` short-circuits, leaving the False
that admitted the final pass and mislabelling a shard that converged on
exactly pass max_iter. So the cap is raised by one and the loop is cut off
from inside the check -- one extra question, not an extra pass.

BREAKING CHANGE: --pycg-shard-timeout is removed. It is the defect, and
after this change it controls nothing. Use --pycg-max-iter to trade recall
against runtime, deterministically. Note that --pycg-max-iter -1 now has no
wall-clock net behind it, so a divergent shard can run indefinitely there.

Closes #145
Exhausting --pycg-max-iter meant "runaway", so the shard was re-partitioned
and re-analysed. That is backwards: hitting the cap means PyCG returned a
sound under-approximation, and splitting such a shard makes the answer
worse, because every cut severs the calls crossing it.

Measured on one 100-file shard of a 2,364-file ORM-heavy project: bounding
the fixpoint and keeping the shard whole gave 110,490 edges in 95s, where
budget-driven halving gave 15,468 edges in 600s. Re-splitting also pays
whole extra rounds of re-analysis -- with a low --pycg-max-iter every shard
hits the cap, and one such run took 2h50m without finishing.

A capped shard now contributes its edges directly; only a shard that raised
is decomposed. Both classifications remain pure functions of the input, so
the reproducibility 55965a4 restored is unaffected.

Also documents that lowering --pycg-max-iter does not reliably bound
runtime (per-pass cost dominates: a 26-file shard needs 5 passes and yields
the identical 93 edges at 3 and at 50), adds a README section explaining
why sharding exists and what it costs, and regenerates the --help block,
which still advertised the --pycg-shard-timeout flag 55965a4 removed.

Scope: this removes the load-dependent shard-dropping mechanism, the 11%
effect #145 was filed for. Output is not yet byte-identical across runs --
a separate, ~100x smaller source remains in Jedi's overload resolution for
open(), tracked as #146.

Refs #145, #146
Two corrections to claims made in this branch.

Timings: the same 100-file shard, same --pycg-max-iter, same fence and same
machine, has completed in ~8 minutes on one run and failed to complete in
81 minutes on another (98% CPU throughout, resident set flat, no output).
PyCG's cost on this workload is bimodal, not merely noisy, so the wall-clock
figures previously quoted (95s / 600s) are not measurements anyone should
rely on -- and the halving figure was a harness cap rather than a completion.
The argument for keeping a capped shard whole rests on edge counts, which are
deterministic properties of the analysis: 110,490 edges whole versus 15,468
halved, because every cut severs the calls crossing it. That claim stands
without any timing.

Termination: 55965a4 argued the wall-clock bound was redundant because
--pycg-max-iter already guarantees termination. That is wrong. The cap bounds
fixpoint *iterations* and is only consulted at pass boundaries, so a single
pathological pass escapes it entirely -- which is what the 81-minute run was
doing. Removing the timeout removes the only wall-clock bound that existed.
The timeout had to go because it made the output depend on machine load, but
nothing replaces it, and the docs now say so instead of implying otherwise.

Refs #145
Reverts 98ecf05 ("fix(pycg): keep capped shards instead of re-splitting
them"). Its supporting measurement was confounded: the whole-shard run
completed three fixpoint passes while the halved run was cut off by the
measurement harness after one, so the comparison spanned different
analysis depths. A depth-matched control favored re-splitting. The whole
series also ran with an unpinned hash seed — _pin_hash_seed() re-execs
only when argv0 is the CLI, and the probes imported the library — so
none of its absolute numbers are reliable. Without evidence the change
does not stand: a capped shard is re-partitioned again, and only a shard
that cannot be split further keeps its partial edges.

Also rewrites the #145 changelog entry: drops seed-tainted timing
figures, removes the claim that --pycg-max-iter guarantees termination,
and links #148 (deterministic Jedi-only fallback for expensive shards).
@rahlk

rahlk commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Closing unmerged: the 2026-08-25 design session decided to remove PyCG entirely rather than fix its shard-outcome semantics — see docs/design/specs/2026-08-25-defuse-linker-call-graph-design.md (committed on feat/issue-148-jedi-only-call-graph) and the repurposed #148. The determinism analysis in this PR (wall-clock shard dropping, the 11% spread, the max_iter termination caveat, the seed-pinning probe trap) remains the evidence base for that decision; the branch commits stay in the successor branch's history.

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.

Sharded PyCG silently drops timed-out shards: call graph is load-dependent and not self-describing

1 participant