[
https://issues.apache.org/jira/browse/WICKET-7202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111248#comment-18111248
]
Pedro Santos commented on WICKET-7202:
--------------------------------------
I implemented the filter on the Wicket side and pushed it to a branch.
CdiConfiguration
now takes a Predicate<Class<?>>:
new CdiConfiguration()
.setInjectionCandidateFilter(c -> c.getName().startsWith("de.custom."))
.configure(application);
ComponentInjector, BehaviorInjector and SessionInjector consult it before
entering CDI.
The default accepts every class, so nothing changes for existing applications.
I left out is the cache. Whether caching is needed at all, and what
should be cached, depends on the container: Liberty deployment clearly needs it,
a Wicket application on a servlet container does not. The change just adds a
place to say
"do not call CDI for this class" and to apply that decision
consistently to components, behaviors and sessions. Once we also ship a cache
we own
the invalidation, the redeploy behaviour and the classloader lifetime of
whatever we
key it on.
That also explains why our profiler results disagreed. I was running Weld on a
bare
Tomcat, where no integrator registers an InjectionServices, so Weld's
aroundInject hook
does nothing and the BeanManager lookups dominate everything else - which is why
WICKET-7126 looked like the whole story to me. On a full Jakarta EE container
like
Liberty the integrator does register one, and Weld calls it on every
InjectionTarget#inject regardless of whether the class has anything to inject.
The InjectionTarget is already
cached per class, by NonContextual, so Weld is not rescanning components. The
negative result is cached too - it is just an empty injection plan. What
repeats per
instance is the ceremony around it: two BeanManager lookups, a
CreationalContext, and
the integrator's aroundInject.
Two things any filter has to get right, and I put them in the javadoc: it must
also
account for @Resource, @PersistenceContext, @PersistenceUnit, @EJB and
@WebServiceRef,
and for @PostConstruct on a Session, because rejecting a class skips those too.
None of
them are reported by InjectionTarget#getInjectionPoints, so a filter that
rejects too
much fails silently. An @Inject-only test is fine as a local optimization when
you know
your own classes.
Tests cover the filter on components, behaviors and sessions, that wicket-cdi's
own
listeners and the application are never filtered, and that CdiShutdownCleaner
still runs.
I used Claude's help to dig through the Weld sources and to measure this.
> wicket-cdi performs non-contextual CDI injection for every Component and
> Behavior even when no injection points exist
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-7202
> URL: https://issues.apache.org/jira/browse/WICKET-7202
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-cdi
> Affects Versions: 10.10.0, 10.11.0
> Reporter: Korbinian Bachl
> Priority: Major
> Attachments: FastCdiComponentInjector.java
>
>
> *Abstract:*
> Wicket’s default CDI integration unconditionally processes every newly
> created component and behavior, even classes without any @Inject members.
> This caused significant CDI overhead and contention in Open Liberty.
> We replaced Wicket’s ComponentInjector and BehaviorInjector with optimized
> listeners. A per-class ClassValue cache determines whether the class or one
> of its superclasses contains injectable members.
> CDI is invoked only when injection is actually required. The BeanManager and
> InjectionTarget instances are also cached and invalidated on CDI or
> redeployment failures.
> As a result, ordinary Wicket components and behaviors bypass CDI entirely,
> while injection remains available for application classes that actually
> require it.
> *Example:*
> enclosed is our FastCdiComponentInjector class only cleaned up with out
> company name that we use; beside this we also use a FastCdiBehaviorInjector
> and AbstractCdiWebApplication but this should give an impression how we
> solved this that CDI inject is no longer a burdon on the app server itself
> *Long Description:*
> CdiConfiguration installs ComponentInjector and BehaviorInjector as global
> instantiation listeners.
> Both listeners unconditionally call AbstractInjector.inject(instance) for
> every
> new Wicket Component and Behavior:
> ComponentInjector / BehaviorInjector
> -> AbstractInjector.inject()
> -> NonContextual.of(instance)
> -> NonContextual.inject(instance)
> This path is also executed for classes such as Label, WebMarkupContainer,
> AttributeModifier and application components which do not declare any CDI
> injection points.
> NonContextual caches the InjectionTarget per BeanManager and class, but it
> does
> not cache or use the negative result "this class has no injection points".
> Therefore every instance still causes:
> * BeanManagerLookup.lookup() while resolving the NonContextual cache;
> * another BeanManagerLookup.lookup() in NonContextual.inject();
> * creation of a CreationalContext;
> * invocation of InjectionTarget.inject().
> BeanManagerLookup caches the last successful lookup strategy, but not the
> BeanManager itself. Thus the successful JNDI or CDI.current() lookup strategy
> is
> still invoked repeatedly.
> In a component-heavy application this creates a very hot CDI path even though
> the overwhelming majority of Wicket framework components and behaviors do not
> contain injectable members.
> We observed severe CDI overhead and container contention with Open Liberty.
> The
> CDI resolution/injection path could not keep up with the rate at which Wicket
> created small Components and Behaviors.
> Environment where the problem was first addressed:
> * Apache Wicket 10.8.0
> * Open Liberty 26.0.0.2
> * Jakarta EE 10
> * Java 25
> The same code path is still present with:
> * Apache Wicket 10.10.0
> * Open Liberty 26.0.0.8
> * Jakarta EE 11
> * Java 25
> Workaround used in the application
> After calling CdiConfiguration.configure(application), we remove Wicket's
> ComponentInjector and BehaviorInjector and replace them with application
> listeners.
> The replacement listeners use the following fast path:
> 1. Exclude known framework/non-application packages.
> 2. Use ClassValue<Boolean> to cache whether the class hierarchy declares
> jakarta.inject.Inject or javax.inject.Inject fields/methods.
> 3. Return without entering CDI when no injectable member exists.
> 4. Cache the BeanManager.
> 5. Cache InjectionTarget instances by target class.
> 6. Create a CreationalContext and invoke InjectionTarget.inject() only for
> classes which actually require injection.
> 7. Invalidate the BeanManager and InjectionTarget caches after container
> failures so that redeployment can recover.
> The same mechanism is used for Components and Behaviors.
> This removes CDI completely from the instantiation path of ordinary Wicket
> framework objects and application objects without injection points.
> Proposed upstream solution
> Please add a class-level injection candidate cache to wicket-cdi and avoid
> calling NonContextual.of(...)/InjectionTarget.inject(...) for Components and
> Behaviors which cannot contain CDI injection points.
> A possible implementation is:
> * Introduce a ClassValue<Boolean> in AbstractInjector or in a small shared
> injection metadata helper.
> * On first use, inspect the complete class hierarchy for injectable fields and
> initializer methods.
> * If the cached result is false, return immediately before BeanManager lookup,
> CreationalContext creation and InjectionTarget invocation.
> * Use the optimized method from both ComponentInjector and BehaviorInjector.
> * Keep SessionInjector/application postConstruct handling unchanged, because
> these objects may have lifecycle callbacks even without injection fields.
> * Cache the resolved BeanManager for the application lifecycle, or retain the
> BeanManager in NonContextual together with its InjectionTarget.
> * Clear container-specific caches during
> CdiShutdownCleaner/NonContextual.undeploy.
> An application package whitelist is a useful local optimization, but should
> not
> be hard-coded upstream. Wicket could instead offer an optional
> Predicate<Class<?>>/injection candidate filter on CdiConfiguration. The
> default
> implementation should at least cheaply reject Wicket framework classes and
> classes without supported injection annotations.
> Compatibility must be considered if wicket-cdi intends to support Java EE
> resource injection annotations other than @Inject. In that case, either those
> annotations must be included in the candidate check, or the fast filter should
> be configurable/opt-in. The application workaround intentionally targets
> @Inject-based CDI injection.
> Acceptance criteria
> * Instantiating a Component without injection points does not create a
> CreationalContext and does not call InjectionTarget.inject().
> * The same applies to Behavior instances without injection points.
> * Field and initializer-method injection still works.
> * Inherited injection points still work.
> * Qualifiers and normal CDI scopes remain unaffected.
> * Session and application @PostConstruct behavior remains unchanged.
> * Metadata caching is safe across concurrent requests and application
> undeployment/redeployment.
> * Tests verify CDI call counts for many instances of the same non-injectable
> Component and Behavior.
> Suggested reproducer
> Create a Wicket quickstart with CdiConfiguration and an instrumented/fake
> BeanManager. Instantiate a page containing several thousand Labels,
> WebMarkupContainers and AttributeModifiers.
> After application initialization, count:
> * BeanManager lookups;
> * createCreationalContext() calls;
> * InjectionTarget.inject() calls.
> Repeat with one Component and one Behavior containing an inherited @Inject
> field to prove that real injection continues to work.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
