Skip to content

feat(core)!: weave without creating aspects - #776

Merged
lisachenko merged 7 commits into
masterfrom
feat/weave-without-aspect-instances
Oct 25, 2026
Merged

lisachenko merged 7 commits into
masterfrom
feat/weave-without-aspect-instances

Conversation

@lisachenko

@lisachenko lisachenko commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

What and why

Weaving used to collect aspects as objects. AspectLoader asked the container for every Aspect service, and AttributeAspectLoaderExtension bound each advice method to that instance (getClosure($aspect)). A deferred aspect stayed an uninitialized lazy proxy, but PHP can't make a class without instance properties lazy. The factory of such an aspect therefore ran as soon as weaving started, before an application container attached with setDelegate() existed. Integration bridges then had to either create the aspect behind the application container's back or fail. The Yii3 bridge hit this in debug mode, where the container loads classes while it's being built: goaop/goaop-yii3-bridge#3.

Aspects are now loaded by class, so weaving, warm-up and the advisor cache never create one:

  • AspectContainer::getServiceIdsByInterface() (new) lists service ids without creating the services.
  • AspectLoaderInterface (all methods @internal): getUnloadedAspects() returns one ReflectionClass per unloaded aspect, and load() / loadAndRegister() take it, so the loaders and extensions share one reflection.
  • AspectLoaderExtension::load() takes the ReflectionClass of the aspect only, and so do AbstractAspectLoaderExtension::parsePointcut() and AttributeAspectLoaderExtension::getAdvice().
  • An interceptor takes Closure|array{class-string<Aspect>, string}. Given the pair, it resolves the aspect itself and keeps the pair in aspectMethod. AttributeAspectLoaderExtension builds advices with the lazy form of the Interceptor facade, the same form compiled advisor caches already use.
  • NativeLazyProxy::create() takes the properties to set on the proxy without initializing it. The lazy interceptors expose adviceOrder, pointcutExpression and aspectMethod, so ordering, GeneratedInterceptor::fromAdvice() and compileToPhp() read them without creating the aspect.
  • debug:aspect reads aspects from their classes too.

Every aspect is now created on the first call of one of its advices (The::aspect()), including aspects without instance properties. Generated code and advised calls are unchanged: proxies keep the eager The::aspect(X::class)->method(...) form.

Breaking changes (4.0 is unreleased; recorded in CHANGELOG.md, not in the upgrade guide): the extension signatures above, a new AspectContainer method for custom containerClass implementations, and aspects must be registered under their class name (generated code already resolves them that way).

Testing:

  • New tests: ContainerTest::testServiceIdsByInterfaceNeverCreateServices; InterceptorTest::testLazyInterceptorDescribesItsAdviceWithoutCreatingTheAspect; AttributeAspectLoaderExtensionTest::testLoadsAdvicesWithoutCreatingTheAspect, with an aspect whose constructor throws; AspectLoaderTest::testDeferredAspectsAreLoadedWithoutCreatingThem; AdvisorIndexTest::testLoadsDeferredAspectsWithoutRunningTheirFactories, end to end with a property-less aspect whose factory throws; NativeLazyProxyTest::testTrustedCreateSetsRawValuesWithoutInitializing, covering an inherited private(set) property.
  • vendor/bin/phpunit: 3253 tests OK (PHP 8.5.11 with opcache.jit=disable, see the JIT note in tests/AGENTS.md). composer analyze and composer cs are clean.
  • Performance: no measurable change in booting or weaving, see feat(core)!: weave without creating aspects #776 (comment).

Follow-up: once this lands, the Yii3 bridge drops its "stateless aspect needed before the container is attached" error (goaop/goaop-yii3-bridge#4).

Checklist

  • The title follows Conventional Commits (type(scope): summary)
  • Tests cover the change
  • composer check passes
  • User-facing or breaking changes are in CHANGELOG.md

Generated by Claude Code

The weaver collected aspects as objects: AspectLoader asked the container
for every Aspect service, and AttributeAspectLoaderExtension bound each
advice method to that instance. A deferred aspect stayed an uninitialized
lazy proxy, but PHP can not make a class without instance properties lazy,
so the factory of such an aspect ran as soon as weaving started, before an
application container attached with setDelegate() existed. Integration
bridges had to create it behind the back of the application container or
fail (goaop/goaop-yii3-bridge#3).

Aspects are now loaded by class. AspectContainer::getServiceIdsByInterface()
lists the aspect ids without creating the services, loader extensions get
the ReflectionClass only, and advices are built with the lazy form of the
Interceptor facade. Its proxies carry the aspect class, the advice method,
the order and the expression, which ordering, GeneratedInterceptor and the
advisor cache compiler read without initializing them. Every aspect is
created on the first call of one of its advices.

BREAKING CHANGE: AspectLoaderExtension::load() takes the ReflectionClass of
the aspect only, AbstractAspectLoaderExtension::parsePointcut() the aspect
class name, AspectLoaderInterface works with aspect class names, and
AspectContainer implementations must provide getServiceIdsByInterface().
Aspects must be registered under their class name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.22%. Comparing base (04e13a9) to head (0479d80).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #776      +/-   ##
============================================
+ Coverage     98.20%   98.22%   +0.02%     
- Complexity     1913     1924      +11     
============================================
  Files           113      113              
  Lines          5000     5024      +24     
============================================
+ Hits           4910     4935      +25     
+ Misses           90       89       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lisachenko

Copy link
Copy Markdown
Member Author

what is performance impact? of booting/weaving?

Copy link
Copy Markdown
Member Author

Measured both, master 04e13a9 vs. this branch 0f33c8f, same machine, runs interleaved. No measurable impact on booting or weaving; the differences are within run-to-run noise.

1. Aspect loading (the part this PR changes): a script that registers N generated aspects with addLazyService() (4 advices each: before/after/around/afterThrowing) and loads them through the real AspectLoader + AttributeAspectLoaderExtension, no advisor cache. Median of 40 rounds, three runs per side:

master branch
10 aspects (40 advices) 2.42 / 2.50 / 2.67 ms 2.50 / 2.70 / 2.49 ms
50 aspects (200 advices) 12.6 / 13.6 / 15.2 ms 13.2 / 13.1 / 12.8 ms

It's about 60 µs per aspect either way, dominated by pointcut parsing. The lazy interceptor plus its 4 raw property writes cost about the same as materializing the aspect and binding a closure did. The test aspects have no properties, so master ran their factories here too; with real aspects that have heavy constructors, master paid that cost during weaving and the branch doesn't.

2. End to end: the goaop-yii3-bridge boot benchmarks (benchmarks/run.php, 50 services, 10 aspects; 30 CLI processes / 500 server requests per scenario) with vendor/goaop/framework switched between the two commits. Run order: master, branch, master. Overhead is the median total minus the no-bridge baseline:

Mode Scenario master (run 1) branch master (run 2)
CLI, cold AOP cache (weaving every class) no aspects +36.1 ms +38.5 ms +38.0 ms
10 aspects +108.3 ms +113.9 ms +124.9 ms
php -S, opcache + AOP cache hot (boot) no aspects +0.31 ms +0.34 ms +0.27 ms
10 aspects +2.95 ms +3.09 ms +2.91 ms

The hot path can't change by design: a warm request never runs the aspect loader, and generated proxies keep the same eager The::aspect(X::class)->method(...) code. CI's Transformation performance (head vs base) job passes as well.


Generated by Claude Code

Comment thread src/Aop/Framework/GeneratedInterceptor.php Outdated
Comment thread src/Aop/Framework/Interceptor.php Outdated
Comment thread src/Aop/AGENTS.md Outdated
Comment thread src/Core/Cache/CachedAspectLoader.php Outdated
Comment thread src/Core/Cache/CachedAspectLoader.php Outdated
Comment thread src/Core/AspectLoaderInterface.php Outdated
Comment thread UPGRADE-4.0.md Outdated
Comment thread src/Aop/Framework/Interceptor.php Outdated
Comment thread src/Aop/Framework/AbstractInterceptor.php Outdated
Comment thread src/Aop/Framework/AbstractInterceptor.php Outdated
…thod]

The interceptor constructor takes the advice closure or the pair of aspect
class and advice method, and keeps the pair in one aspectMethod property.
NativeLazyProxy::create() sets the properties the lazy facade exposes, so
the facade no longer handles reflection properties.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe
getUnloadedAspects() returns one ReflectionClass per aspect, and load() and
loadAndRegister() take it, so the loaders no longer reflect the same aspect
again. The methods of AspectLoaderInterface are marked internal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe
Comment thread src/Core/AttributeAspectLoaderExtension.php Outdated
Comment thread src/Core/AttributeAspectLoaderExtension.php Outdated
…Advice()

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe
Comment thread UPGRADE-4.0.md Outdated
@lisachenko
lisachenko marked this pull request as ready for review October 25, 2026 22:16
@lisachenko
lisachenko merged commit 1073ae8 into master Oct 25, 2026
14 checks passed
@lisachenko
lisachenko deleted the feat/weave-without-aspect-instances branch October 25, 2026 22:17
lisachenko pushed a commit to goaop/goaop-yii3-bridge that referenced this pull request Oct 25, 2026
… exists

goaop/framework#776 weaves without creating aspects, so an aspect without
instance properties is no longer needed while the validating container is
built: it is the instance of the Yii container in debug mode too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgdBdi3AXZ33wvVCFG3GPe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants