Skip to content

perf(engine): hoist O(N^2) static-ability scans out of combat/untap legality loops#4445

Merged
matthewevans merged 1 commit into
mainfrom
perf/combat-static-scan-hoist
Jun 27, 2026
Merged

perf(engine): hoist O(N^2) static-ability scans out of combat/untap legality loops#4445
matthewevans merged 1 commit into
mainfrom
perf/combat-static-scan-hoist

Conversation

@matthewevans

Copy link
Copy Markdown
Member

Problem

On a large token board (Unbeatable Squirrel Girl: 702 Squirrels + Cryptolith Rite, ~760 battlefield permanents), declaring attackers and passing priority crawled. Root cause: several combat/untap legality operations are O(N²) in battlefield size — each loops over N permanents and calls check_static_ability (itself an O(N) game_functioning_statics sweep) per element.

GameState uses im persistent collections, so cloning is O(1) — the cost is the repeated full-battlefield static scans, not memory.

Fix — early-gating (byte-identical)

Hoist a single "does any functioning static of MODE exist?" existence check before each loop, and gate the per-element scan behind it. Because check_static_ability returns false when no static of that mode exists (it continues on def.mode != mode), gate && check_static_ability(MODE, ctx) is byte-identical to the original call: when the gate is false the call would have returned false anyway; when true, the exact original call runs with its full CR gate stack intact (affected filter, condition, attack_defended scope CR 508.1d, per_player_condition CR 101.2/109.5). Precompute was rejected precisely because it would force re-implementing those matchers.

No verdict changes. No serialized-state change.

New building blocks (engine only)

  • any_functioning_static_mode — the loop-invariant existence gate primitive (CR 604.1).
  • CombatStaticGates::compute — one game_functioning_statics sweep → 5 attack-restriction existence flags, shared across the attacker seams.
  • goading_players_for_creature_gated / gated must-attack helper — skip the goad sweep when no Goaded static exists, still returning the base goaded_by set (CR 701.15b).
  • static_full_scans perf counter — incremented at the sole scan authority (check_static_ability), so revert-failing tests assert the per-element scan count stays 0 on restriction-free boards.

Gated seams

get_valid_attacker_ids, has_potential_attackers, validate_attackers, declare_attackers_with_bands (must-attack / scoped CantAttack / goad redirect), validate_blockers_for_player (MustBlock), execute_untap_with_choices + untap_excluded_ids (CantUntap), and a max_untap_subset_prompt early-bail. The untap path was the every-turn O(N²) — its regression guard drives the production execute_untap, not a prompt helper.

Tests

Revert-failing static_full_scans == 0 guards per seam, plus gate=true behavior tests (scoped/per-player CantAttack, MustBlock, CantUntap held tapped with scans > 0, goad redirect) proving no restriction is dropped.

Verification: cargo fmt clean, clippy -p engine --all-targets clean, cargo test -p engine --lib = 13690 passed / 0 failed.

Deferred (Tier-2 follow-ups)

  • blocker_can_block_shadow (CanBlockShadow — pairwise/cold path, needs a different precompute shape).
  • The AI single-permanent creature_must_attack wrapper (recomputes gates per call in AI enumerators; strictly faster than pre-fix, not a regression).
  • Separately found O(N²): auto-pass legal_actions re-enumeration, taps_for_mana_aura_bonus, crew/saddle/station eligibility, expand_granted_activated_abilities.

🤖 Generated with Claude Code

…egality loops

Combat attacker eligibility, attacker declaration, block-requirement, and
the every-turn untap step each looped over N battlefield permanents calling
check_static_ability (itself an O(N) game_functioning_statics sweep) per
element -> O(N^2). On a large token board (Unbeatable Squirrel Girl: 702
Squirrels + Cryptolith Rite) this made declaring attackers and passing
priority crawl.

Fix via early-gating: hoist a single 'does any functioning static of MODE
exist?' existence check before each loop and gate the per-element scan
behind it. Because check_static_ability returns false when no static of
that mode exists (it skips on def.mode != mode), 'gate && check_static_ability'
is byte-identical to the original call, and when the gate is true the exact
original call runs with its full CR gate stack (affected filter, condition,
attack_defended scope CR 508.1d, per_player_condition CR 101.2/109.5) intact.
No verdict changes; no serialized-state change.

New building blocks (engine only):
- any_functioning_static_mode (functioning_abilities.rs): the loop-invariant
  existence gate primitive (CR 604.1).
- CombatStaticGates::compute (combat.rs): one game_functioning_statics sweep
  -> 5 attack-restriction existence flags, reused across the attacker sites.
- goading_players_for_creature_gated / gated must-attack helper: skip the
  battlefield_active_statics goad sweep when no Goaded static exists, still
  returning the base goaded_by set (CR 701.15b).
- static_full_scans perf counter (perf_counters.rs), incremented at the sole
  scan authority check_static_ability, so revert-failing tests can assert the
  per-element scan count stays 0 on restriction-free boards.

Gated seams: get_valid_attacker_ids, has_potential_attackers, validate_attackers,
declare_attackers_with_bands (must-attack / scoped CantAttack / goad redirect),
validate_blockers_for_player (MustBlock), execute_untap_with_choices and
untap_excluded_ids (CantUntap), plus a max_untap_subset_prompt early-bail.

Tests: revert-failing static_full_scans==0 guards per seam (the untap guard
drives the production execute_untap path), plus gate=true behavior tests
(scoped/per-player CantAttack, MustBlock, CantUntap held tapped with scans>0,
goad redirect) proving no restriction is dropped.

Deferred to Tier-2: blocker_can_block_shadow (CanBlockShadow, pairwise/cold)
and the AI single-permanent creature_must_attack wrapper.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@matthewevans matthewevans enabled auto-merge June 26, 2026 23:42
@matthewevans matthewevans added this pull request to the merge queue Jun 26, 2026
Merged via the queue into main with commit b45ee51 Jun 27, 2026
11 checks passed
@matthewevans matthewevans deleted the perf/combat-static-scan-hoist branch June 27, 2026 00:03
Whovencroft pushed a commit to Whovencroft/phase that referenced this pull request Jun 27, 2026
…/crew/combat paths (phase-rs#4448)

Tier-2 follow-up to the combat/untap static-scan hoist (phase-rs#4445). Four
board-composition-specific O(N^2) sites collapsed to O(N) by hoisting a
loop-invariant, state-only computation out of a per-battlefield-permanent
loop. All four are byte-identical (pure perf hoists); each adds a
thread-local perf counter and a revert-failing test asserting the hoisted
work runs O(1) times instead of O(N).

A: auto-pass meaningful-action probe (engine.rs) now calls a new
   flat_priority_actions() single-authority body instead of legal_actions,
   dropping the unused spell-cost object-walk + grouped map. legal_actions_full
   wraps the same body. Both probe sites are waiting_for==Priority, so the
   target-selection path is None and the result is byte-identical. CR 117.1.

B: board-global auto-tap / activatable mana sweeps hoist
   taps_for_mana_trigger_sources() once and thread Option<&[ObjectId]> through
   land_mana_options (None = per-land compute for display/single/test callers,
   byte-identical). Preserves the land self-skip, per-source card-identity
   check, and the deliberate no-controller-filter (Aura-Theft). CR 605.1b/106.12a.

C: AI crew/saddle/station candidate generation precomputes untapped_creatures
   and crew_eligible (minus object_has_cant_crew) once instead of rescanning
   per Vehicle. Crew uses crew_eligible; Saddle/Station use the base set. The
   cid != obj_id self-exclusion is preserved on all three. CR 702.122a/702.171a/702.184a.

E: AI must-attack partition hoists attackable_player_targets() once instead of
   recomputing it per goaded creature inside creature_must_attack; both AI
   filters call the batched creature_must_attack_with_attackable_players.
   CR 508.1d.

Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
Whovencroft pushed a commit to Whovencroft/phase that referenced this pull request Jun 27, 2026
…anted-ability provider cache (phase-rs#4453)

Final two items (d, f) of the loop-invariant-hoist perf campaign
(phase-rs#4437/phase-rs#4445/phase-rs#4448). Both byte-identical (pure hoists/memoization, no rules
change); each adds a thread-local perf counter + a revert-failing test driving
a production entry point, plus equivalence/hostile siblings.

f: blocker_can_block_shadow ran a full game_functioning_statics sweep per
   blocker inside per-attacker x per-blocker block-legality loops
   (O(attackers x blockers x N) on shadow boards). Hoist a once-computed
   CanBlockShadow existence gate (any_functioning_static_mode) per loop-owner
   and thread it through a new blocker_can_block_shadow_gated arity. Gate-false
   == the full predicate is already false (the gate's game_functioning_statics
   universe is a strict superset of both sub-checks), so byte-identical. Threaded
   through all engine prod + test callers of can_block_pair_with_precomputed and
   the phase-ai BlockLegalitySlices mirror struct. CR 509.1b/609.4/702.28b/604.1.

d: expand_granted_activated_abilities re-filtered ALL objects per battlefield
   recipient (O(recipients x objects)) for GrantAllActivatedAbilitiesOf hosts
   (Agatha's Soul Cauldron class). The provider-match context depends only on
   the recipient's controller (from_source_with_controller), not its identity,
   so memoize the matching-provider set per controller in a
   HashMap<PlayerId, Vec<ObjectId>> -> O(controllers x objects). Sorted
   provider order, per-recipient self-skip at emission, and next_mod_index are
   preserved, so emission order and every mod_index are byte-identical.
   CR 109.5/604.1/613.1f.

Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
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