Skip to content

perf: batch affect_total in CastAffect (#3106)#3118

Merged
bylins merged 1 commit into
masterfrom
perf/affect-total-batch-recalc
Apr 10, 2026
Merged

perf: batch affect_total in CastAffect (#3106)#3118
bylins merged 1 commit into
masterfrom
perf/affect-total-batch-recalc

Conversation

@bylins
Copy link
Copy Markdown
Owner

@bylins bylins commented Apr 10, 2026

Summary

  • Add ImposeAffectNoRecalc — variant of ImposeAffect that skips affect_total() recalculation
  • Update CastAffect loop to use ImposeAffectNoRecalc, call affect_total(victim) once after the loop
  • Reduces N calls to affect_total per spell to exactly 1

Context

Profiler data from #3106 shows affect_total() takes ~6-10ms per call (iterates 20 equipment slots × 8 obj affects × ~48 weapon affects + character affects). CastAffect called ImposeAffect up to 16 times in a loop, each triggering a full affect_total recalculation.

Step 'Punctual' took 0.009294 second(s) (90.526069%);
Step 'main_hit' took 0.005989-0.010408 second(s) (98-99%);

Test plan

  • Builds without errors
  • Verify spell affects still apply correctly (duration, modifier, bitvector)
  • Compare profiler output before/after for hit() and PlayerAttack
  • Stress test with multiple casters in combat

🤖 Generated with Claude Code

affect_total() is expensive (~6-10ms with full equipment and 30+ affects)
because it iterates all equipment slots × object affects × weapon affects.
Previously CastAffect called ImposeAffect in a loop (up to 16 iterations),
and each ImposeAffect called affect_total via affect_to_char. This meant
N calls to affect_total per spell cast.

Add ImposeAffectNoRecalc / affect_to_char_no_recalc variants that skip
the affect_total recalculation. CastAffect now uses these and calls
affect_total once after the loop.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bylins bylins merged commit 0305973 into master Apr 10, 2026
20 checks passed
@bylins bylins deleted the perf/affect-total-batch-recalc branch April 10, 2026 15:53
@bylins bylins mentioned this pull request Apr 24, 2026
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