Skip to content

Loadout Optimizer: remove optimizations that measured at noise - #11877

Closed
delphiactual wants to merge 3 commits into
lo-process-readabilityfrom
lo-remove-nonpaying-opts
Closed

Loadout Optimizer: remove optimizations that measured at noise#11877
delphiactual wants to merge 3 commits into
lo-process-readabilityfrom
lo-remove-nonpaying-opts

Conversation

@delphiactual

Copy link
Copy Markdown
Contributor

Requires #11874

Follow-up to the review feedback on #11868 about undoing optimizations that aren't meaningful. I ran an ablation study over the LO worker optimizations: each one was toggled off individually and benchmarked interleaved against the full configuration in the same run (5 rounds, min-over-rounds, so clock drift can't skew ratios), on synthetic corpora and a real vault profile, with the parity suite verifying every ablated configuration still produces identical results. Four optimizations measured at or below noise (0.96x to 1.05x) in every scenario, so this removes them, one commit each:

  • Remove the max-boost memo from updateMaxStats. The internal MAX_STAT and single-stat upper-bound checks already keep the binary search rare, and chooseAutoMods memoizes the underlying solves, so the second memo layer never paid for itself. Also lets buildContextKey/packStatNeeds go private to auto-stat-mod-utils.
  • Remove the per-set energy assignment cache. updateMaxStats and pickOptimalStatMods rarely both run for the same set, so the shared SetEnergyCache almost never got a hit, including in the locked-activity-mods scenario it was built for.
  • Drop the convergence gate before updateMaxStats. It mirrored conditions the function already enforces internally.
  • Un-unroll the six-stat adds. The manual unrolling stopped measuring once the adds moved into a small helper; the plain loop reads better.

Combined verification: parity and all loadout-builder tests pass, and the A/B bench against the frozen baseline is unchanged with all four removed together (15.9x/2.5x/10.0x vs the 15.0x/2.4x/10.2x reference on the same code shape), so there's no pairwise interaction between the removals.

An ablation benchmark measured this memo at 0.96-1.01x across synthetic and real-vault scenarios: the convergence gate and the cheap upper-bound check already keep the binary search rare, and chooseAutoMods memoizes the underlying solves, so the second memo layer never paid for itself. buildContextKey and packStatNeeds become private to auto-stat-mod-utils.
An ablation benchmark measured the SetEnergyCache at 0.99-1.03x across every scenario, including the locked-activity-mods case it was built for: in practice updateMaxStats and pickOptimalStatMods rarely both run for the same set, so the shared cache almost never gets a hit. Callers just compute the energy assignments where needed.
An ablation benchmark measured this pre-check at 0.97-1.05x everywhere: updateMaxStats already skips converged stats internally via the MAX_STAT check and the cheap single-stat upper bound, so the duplicated external mirror of those conditions wasn't buying anything.
@delphiactual

Copy link
Copy Markdown
Contributor Author

measuring in node only is not great, building a playwright test harness...

@delphiactual
delphiactual deleted the lo-remove-nonpaying-opts branch July 10, 2026 13:14
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