choosing-what-to-optimize

name

choosing-what-to-optimize

description

Decide what is worth making faster in this repo, and prove it paid — measure the thing instead of grepping for it, separate wall clock from total CPU from the critical path, rank targets by shape rather than by how many modules import them, and decline the work with a number when the number says so. Use this whenever someone asks to speed up the tests, the sweep, the gates, CI or a hook; when a profile or a grep suggests a hot spot; when a refactor is justified by "this file is bad" rather than by a measurement; and before deferring or closing a performance item — even when the request sounds like ordinary cleanup rather than optimization.

Choosing what to optimize

Everything here was paid for. The figures are from one pass over this repo's own test machinery, and every one of them carries the command that re-derives it, because a number in a document is a number that rots.

The short version: the repo has no cheap 20% lying around. What it has is a handful of suites doing real work against real git and real browsers. So the value of an optimization pass here is mostly in not doing the wrong one.

Measure the thing; do not grep for it

Three findings in one session were reported wrong, each because a grep stood in for a run:

claimed	actual	why the grep lied
38 subprocess calls in a suite	453 child processes	the grep counted textual occurrences; most calls span lines and most spawns come from the code under test
1 tool without a test suite	3	all three printed a pointer containing --selftest, so the grep matched the word and missed the absence
0 section markers in an 8000-line file	42	the lint's own regex allows two spaces of indentation; the grep was anchored at column 0

The third was the expensive one: it was the premise of a plan. The lint would have passed on that file, so the rule being added to force a split would have forced nothing.

Ask the subject, not a proxy for it: run the profiler, call the lint function, count the processes. When a claim is about a rule, ask the rule — import the module and call it rather than reimplementing its pattern in a grep.

Three numbers, not one

"Slow" is three different measurements and they lead to three different decisions.

python3 tools/sweep-selftests.py             # wall clock: what you wait for
python3 tools/sweep-selftests.py --jobs 1    # the serial shape, for a bisect

# total child CPU: what a 2-core CI runner pays, since its wall clock is ~CPU/2
import resource, subprocess, sys, time
b = resource.getrusage(resource.RUSAGE_CHILDREN); t0 = time.time()
subprocess.run([sys.executable, "tools/sweep-selftests.py"], capture_output=True)
a = resource.getrusage(resource.RUSAGE_CHILDREN)
print(time.time() - t0, (a.ru_utime - b.ru_utime) + (a.ru_stime - b.ru_stime))

The sweep measured 11.4s wall against 52.6s of child CPU. Those are not the same fact:

• Wall clock is bounded below by the slowest single file, because each file is one process. Parallelism buys wall clock and nothing else.
• Total CPU is what CI pays. This repo's CI runs the sweep four times (two passes × two operating systems), so CPU is multiplied by four and divided by the runner's cores.
• The critical path is the slowest file. Nothing below it can move the wall clock, however hot it looks in a profile.

A change that halves a file which is not the critical path improves the wall clock by exactly zero. Say which of the three numbers your change moves, before making it.

Do not optimize off the critical path

The worked example is a decision to decline.

_deps._scan_edges parses every .py under scripts/ and grows a wrapper map to a fixpoint. Six files import that graph, and each pays one scan of roughly 0.9s — 8.89s of the sweep's 52.6s of CPU, about 17%. Hoisting the per-tree walk out of the fixpoint would recover most of one scan, so call it ~10.6% of CPU and 0s of wall clock, because the critical path is a different suite entirely.

Against that: the fixpoint decides the runtime-load edges of the import graph, which every layer lint and the whole affected.py selector read. It is the subtlest code in the module, and it carries a written argument for why it must be a fixpoint rather than two passes.

So it was closed as won't-do. Ten percent of CPU, none of the wall clock, on the code that underpins every layer lint, is the worst risk-to-reward ratio available.

And then the rule was broken by the person who had just written it. The "free" version — make a verification block pay for one tree scan instead of three — produced two defects in ten minutes: one seeded a cache from the same value the case compares it against, so the check compared a value with itself and could no longer fail; the other saved nothing and left behind a comment claiming a saving. Both were reverted. The lesson is not that the attempt was careless. It is that a micro-optimization off the critical path has no upside to pay for its mistakes.

Rank by shape; coupling is a cost, not a reason

A composite score that sums shape metrics with coupling ranks the popular, not the bad. Scored that way, the top target came out as the module 80 others import — which had one long function and clean metrics otherwise. It ranked first for being depended upon.

Separate the axes. Score shape only — longest function, functions over N lines, parameter count, repeated literals, branch density per 100 lines — and print fan-in/fan-out beside it as the cost of touching the file. That is the same rule this repo applies to recommending options: effort and blast radius are facts about a choice, never the reason for it.

Then read the file before believing the score. On the re-ranked first place, most of the score did not survive inspection:

• "25 short names" were k, p, t, a/b — loop and comprehension variables. The metric counted 25 distinct names, not 25 bad ones.
• seven of eight "repeated literals" were dict keys ("findings" at 39 sites). That is a key name used at every construction site, not a duplicated fact, and out[_FINDINGS] would read worse at all 39.
• the eighth was a duplicated return value, four byte-identical copies of one refusal. That one was real, and it was one extraction.

A mechanical rank is a pointer, not a verdict. Inspecting the pointer is what turns it into work — and roughly one metric in eight survived that inspection here.

A saving spent is not a saving lost, but you have to report both

The same pass memoised a scan and cut one suite from 16.3s to about 3s. The serial sweep went from 44.0s to 41.9s.

That is not a contradiction and it is not a disappointment: the ~10s the memo returned was spent on 8 more files and 126 more cases in the same pass. Reporting only the suite's 5× would overstate it; reporting only the 2s would understate it. Report the pair — what the change returned, and what the same pass spent — or the number lies in one direction or the other.

Decline with a number

"We did not get to it" and "10.6% of CPU, 0s of wall clock, on the code every layer lint depends on" are different decisions. The first cannot be revisited, because nothing in it says what would have to change. The second can: it names a threshold, so a future reader who finds the critical path elsewhere, or CI paying for eight cores, knows exactly which assumption to re-measure.

Write the deferral as a measurement, and name the trigger that would reopen it.

Where the costs actually are

Re-derive rather than trusting this table — the point of listing the commands is that the shape of the answer changes as the tree grows.

cost	how to see it
the sweep's wall clock, and its slowest file	python3 tools/sweep-selftests.py prints one row per file
total CPU, and therefore CI	the getrusage snippet above
the panel browser gate — by far the longest single leg	node tools/capture-screenshots.mjs --check
the report gate	node tools/check-report-interactive.mjs docs/index.html
which gates a change actually owes	python3 tools/affected.py
whether a lint is worth speeding up at all	python3 tools/prove-gates.py --list

Two standing facts worth knowing before proposing anything: the browser gates dominate the full run, and the sweep's slowest suites are slow because they drive real git and a real browser rather than fakes. That is the correct choice — a fake would encode the assumption instead of the behaviour — so their cost is not a defect to remove.

Checklist

• The claim about the cost came from running the subject, not from a grep or a guess
• Named which of the three numbers the change moves: wall clock, total CPU, critical path
• Confirmed the target is ON the critical path, or said explicitly that only CPU moves
• Ranked by shape, with coupling reported beside it rather than added into it
• Read the top-ranked file before trusting its score
• Reported what the change returned AND what the same pass spent
• A deferral names its number and the trigger that would reopen it
• Re-measured after the change with the same command as before it

Related: no-silent-pass for proving a check can fail before trusting it — including a verification whose shortcut made it circular, which is how the declined optimization above went wrong twice.