Skip to content

Perf: hoist loop-invariant option getters out of the main loop - #61

Merged
mmucklo merged 1 commit into
masterfrom
perf/hoist-loop-invariants
Jul 13, 2026
Merged

Perf: hoist loop-invariant option getters out of the main loop#61
mmucklo merged 1 commit into
masterfrom
perf/hoist-loop-invariants

Conversation

@mmucklo

@mmucklo mmucklo commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Profile-driven follow-up to the mb_str_split change (#60). An Xdebug profile showed the remaining cost concentrated in the state-machine loop body, with ParseOptions getters (getSeparators 1.5%, getBannedChars, getUseWhitespaceAsSeparator) called inside the per-character loop.

Change

Those three getters return immutable config that cannot change during a parse() call, yet they were invoked on every character (a few times per character in the hot states — isset(getSeparators()[$curChar]) etc.). Fetch them once into locals before the loop:

$separators = $this->options->getSeparators();
$bannedChars = $this->options->getBannedChars();
$useWhitespaceAsSeparator = $this->options->getUseWhitespaceAsSeparator();

This removes a method call and its surrounding opcodes from every character iteration — the gain is larger than the getters' own self-time because it also shrinks the enclosing parse() loop-body opcode count.

Result

~19% faster on a mixed-input micro-benchmark (87.8 → 71.0 μs/parse), on top of #60. The benchmark CI job on this PR measures head-vs-master per subject automatically.

Safety

  • Immutable during a parse: no ParseOptions setter runs mid-parse(), so the hoisted values are loop-invariant.
  • Behavior-preserving: all 91 tests pass, PHPStan level 8 / cs clean.

Context: diminishing returns

For transparency — the profile also confirmed the low-hanging fruit is now gone:

  • mb_str_split is only ~0.3% of parse time; an ASCII str_split fast-path measured net-negative (the ASCII-detection check costs more than it saves).
  • idn_to_ascii is ~0.7% (already ASCII-fast-pathed), NFC only runs in intl modes.

Remaining cost is spread across the state machine and per-address object construction. Further gains would need structural changes (e.g. reworking the result-object building) with more risk for less reward.

getSeparators(), getBannedChars(), and getUseWhitespaceAsSeparator() were
called inside the per-character loop (up to a few times per character across
the hot states). They return immutable config that cannot change during a
parse, so they are now fetched once into locals before the loop, removing a
method call and its surrounding opcodes from every character iteration.

~19% faster on a mixed-input micro-benchmark (87.8us -> 71.0us/parse), on top
of the mb_str_split change. No behavioral change — all 91 tests pass.
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.89%. Comparing base (30b9e9a) to head (94dc39f).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #61      +/-   ##
============================================
- Coverage     93.17%   92.89%   -0.29%     
  Complexity      380      380              
============================================
  Files             6        6              
  Lines           982      985       +3     
============================================
  Hits            915      915              
- Misses           67       70       +3     
Files with missing lines Coverage Δ
src/Parse.php 90.11% <100.00%> (-0.40%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mmucklo
mmucklo merged commit df046fa into master Jul 13, 2026
14 checks passed
@mmucklo
mmucklo deleted the perf/hoist-loop-invariants branch July 13, 2026 07:23
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