[ 
https://issues.apache.org/jira/browse/WICKET-7202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110824#comment-18110824
 ] 

Pedro Santos commented on WICKET-7202:
--------------------------------------

Hi, thank you for the new type.

I had a similar problem and while checking the time to reply a request and 
noticed that most of the time was being used was beanmanager lookups. It would 
be helpful if you could pinpoint the exact point the module was taking time 
because the slow down could have the same cause as mine.

In your patch,

private static boolean hasInjectMembers(Class<?> type)

is doing the loading bearing 

at = beanManager.createAnnotatedType(clazz);
it = manager.getInjectionTargetFactory(at).createInjectionTarget(null);
if (

it.getInjectionPoints().isEmpty() &&

at.getMethods().stream() .noneMatch(m -> 
m.isAnnotationPresent(PostConstruct.class))

)

Weld cloud be doing by testing on its own cached data in O(1).

My concern is to add in Wicket a cache that already exists in the CDI 
implemantation and not tackle the beanmanager lookup happening least one time 
in each AbstractInjector point.

I proposed a solution for the slow down in WICKET-7126 [1], let us know if this 
reduced number of lookup is enough or both improvements are needed.

1 - https://issues.apache.org/jira/browse/WICKET-7126

> 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)

Reply via email to