Skip to content

perf(reflection): mint instances via native reflection and drop redundant casts on hot paths - #196

Merged
lisachenko merged 2 commits into
8.4from
claude/php-repo-modernization-6pfu7y-reflection-fastpath
Aug 13, 2026
Merged

perf(reflection): mint instances via native reflection and drop redundant casts on hot paths#196
lisachenko merged 2 commits into
8.4from
claude/php-repo-modernization-6pfu7y-reflection-fastpath

Conversation

@lisachenko

@lisachenko lisachenko commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Two independent, behavior-preserving reductions on reflection hot paths. No public API changes.

Note: the PR originally carried a third part (entry-level static helpers used by countPublishedShares()); it was dropped per maintainer review — consumers hold Reflection* objects, entry-level static utilities cut against the framework's model.

A. Stop building full z-engine ReflectionClass instances just to bypass constructors

Inside namespace ZEngine\Reflection the unqualified name ReflectionClass resolves to z-engine's own class, whose constructor runs the native parent::__construct(), a class-table find() and initLowLevelStructures() (four HashTable wrappers) — every bit of it discarded when the instance exists only to serve newInstanceWithoutConstructor().

All ten such sites now use ReflectionClass as NativeReflectionClass, the alias already used by ReflectionClass::fromCData() (src/Reflection/ReflectionClass.php:183) and the src/Type/* wrappers:

  • src/Reflection/ReflectionMethod.phpfromCData(), fromHookCData(), fromRawEntry(), fromClosureEntry()
  • src/Reflection/ReflectionProperty.phpfromCData(), fromRawEntry()
  • src/Reflection/ReflectionClassConstant.phpfromCData(), fromRawEntry()
  • src/Reflection/ReflectionFunction.phpfromCData(), fromClosureEntry()

Deliberately left unchanged: src/Reflection/ReflectionMethod.php:152, (new ReflectionClass($boardName))->getMethodTable() — that one genuinely needs the z-engine class, since getMethodTable() only exists once initLowLevelStructures() has run. Every other site was checked the same way.

B. Remove redundant zend_class_entry * casts

Each of these passed an already-typed zend_class_entry * through Core::cast('zend_class_entry *', …), which costs a count() array probe, a caught FFI\Exception, and a type-name lookup per call. The producing expression was checked at every site — all four are pointer struct fields (zend_object.ce, zend_function_common.scope), never a C array needing decay — so the cast is dropped, matching the existing direct form in ReflectionProperty::getDeclaringClass() and ReflectionClassConstant::getDeclaringClass():

  • src/Type/ObjectEntry.php:131$this->pointer->ce, zend_object.ce is ?zend_class_entry in the stubs
  • src/Reflection/ReflectionMethod.php:332$this->getCommonPointer()->scope
  • src/Reflection/FunctionLikeTrait.php:284$entryScope, fed from $this->getCommonPointer()->scope at line 198
  • src/Reflection/FunctionBodySwap.php:406$entryFunction->getCommonPointer()->scope

No site had to be left in place for array decay.

Merge note

ReflectionMethod::fromHookCData() is also touched by the concurrently-open exception-safety PR #197. The hunks are different and roughly ten lines apart, so a textual proximity conflict is possible but the resolution is trivial in either merge order — keep both edits.

Validation

  • vendor/bin/phpstan analyse (level max) — clean, no baseline changes needed (verified via a temporary reportUnmatchedIgnoredErrors: true run on both trees).
  • PHP_CS_FIXER_IGNORE_ENV=1 vendor/bin/php-cs-fixer fix — 0 of 296 files changed; --dry-run clean.
  • Local tests were not run and could not be: the container runs PHP 8.5.9 while this branch targets PHP 8.4, and AGENTS.md forbids executing z-engine code against a mismatched minor. Validation here is static-only; CI on the 8.4 legs is the real gate.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG

…dant casts

Three hot-path reductions in the reflection layer, no behavior change.

Instance minting: inside namespace ZEngine\Reflection the unqualified name
ReflectionClass resolves to z-engine's own class, whose constructor runs the
native parent constructor, a class-table find() and initLowLevelStructures()
(four HashTable wrappers) - all discarded when the instance only serves
newInstanceWithoutConstructor(). The ten such sites in ReflectionMethod,
ReflectionProperty, ReflectionClassConstant and ReflectionFunction now use
the aliased native reflector, matching ReflectionClass::fromCData() and the
Type\* wrappers. ReflectionMethod::fromHookCData() keeps the z-engine class
for the publication board, which genuinely needs getMethodTable().

Class-table walk: FunctionBodySwap::countPublishedShares() built a full
ReflectionClass per engine class only to read one type byte and look up one
method bucket. Both answers now come off the entry through the owning class
via new @internal ReflectionClass::entryIsUserDefined() and
ReflectionClass::entryMethodTable() helpers, so no call site pokes the raw
zend_class_entry itself. Counting semantics (alias dedup, user-class filter,
address match, floor of 1) are unchanged.

Redundant casts: four sites passed an already-typed zend_class_entry *
through Core::cast('zend_class_entry *', ...), paying a count() probe, a
caught FFI exception and a type lookup per call. Each value is a pointer
field (zend_object.ce, zend_function_common.scope), never an array needing
decay, so the cast is dropped as in ReflectionProperty::getDeclaringClass().

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
@lisachenko

Copy link
Copy Markdown
Owner Author

Drop B. it is against the idea of framework

countPublishedShares() reflects each class again: the framework's model is
that consumers hold Reflection objects, and entry-level static utilities on
ReflectionClass cut against that. The redundant zend_class_entry* cast
removal in applyStaticVariableDefaults() stays.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG

Copy link
Copy Markdown
Owner Author

Done — Part B is reverted in 8100b30: the entryIsUserDefined()/entryMethodTable() statics are removed and countPublishedShares() builds ReflectionClass::fromCData() per class again, as before. Parts A and C (native reflector for instance minting, redundant cast removal) are unchanged; the PR description is updated accordingly. The concurrently-running hook-consolidation agent was also told not to reuse those statics.

phpstan (level max) and php-cs-fixer re-verified clean after the revert.


Generated by Claude Code

@lisachenko
lisachenko marked this pull request as ready for review August 13, 2026 19:36
@lisachenko
lisachenko merged commit c7d5f90 into 8.4 Aug 13, 2026
19 checks passed
@lisachenko
lisachenko deleted the claude/php-repo-modernization-6pfu7y-reflection-fastpath branch August 13, 2026 19:41
lisachenko pushed a commit that referenced this pull request Aug 13, 2026
Re-bases the CI signal on the true merged state: 8.4 gained the
exception-factory classes (#201) that also touch OpCodeHook's
install-time guards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
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.

2 participants