You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Jörg Hoh <jh...@googlemail.com.INVALID> on 2022/06/22 11:43:51 UTC

Sling Model AdapterFactory - iterating all injectors?

Hi,

While investigating some weirdness in custom Sling Model Injectors I came
across, that a SlingModel triggerd the invocation of the prepare() method
of a totally unrelated Injector, which implements the ValuePreparer
interface.

I found that if no injector is given in the annotation [1] all available
injectors are taken [2], iterated and the ValuePreparer invoked (if it's
implemented by the Injector).

That means that in case no specific injector is given, all injectors are
tried, which can be costly in terms of time. What I miss (and what I think
should be possible) is to reduce this list based on the type information.
Because it does not make sense to query an Injector which can only return a
String if an injection for the type SlingHttpServletRequest should be done.
Would such an optimization be possible?

I don't have benchmark data yet how much time is spent in the process of
injecting all required fields, but before I invest more time here, I wanted
to understand if this is feasible at all.

Thanks,
Jörg


[1]
https://github.com/apache/sling-org-apache-sling-models-impl/blob/ed1bd8d8a2fc01e13a10ba1c06122530c4d590b1/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L574

[2]
https://github.com/apache/sling-org-apache-sling-models-impl/blob/ed1bd8d8a2fc01e13a10ba1c06122530c4d590b1/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L570




-- 
Cheers,
Jörg Hoh,

https://cqdump.joerghoh.de
Twitter: @joerghoh

Re: Sling Model AdapterFactory - iterating all injectors?

Posted by Jörg Hoh <jh...@googlemail.com.INVALID>.
Hi Konrad,

Yes, making it more explicit what injector you expect should help here, I
raised https://issues.apache.org/jira/browse/SLING-11403 . But even then we
will never get rid of the @Inject annotation, people will still use it. Not
to mention the amount of sling models written over the last 5 years, and
many of them are unlikely to be changed/improved.

Jörg

Am Mi., 22. Juni 2022 um 13:47 Uhr schrieb Konrad Windszus <konrad_w@gmx.de
>:

> Hi,
> Shouldn’t we instead just more prominently promote injector-specific
> annotations (
> https://sling.apache.org/documentation/bundles/models.html#injector-specific-annotations
> <
> https://sling.apache.org/documentation/bundles/models.html#injector-specific-annotations
> >)?
> IMHO this should be fixed in the model and we shouldn’t try to be smarter
> with generic injections.
>
> Konrad
>
> > On 22. Jun 2022, at 13:43, Jörg Hoh <jh...@googlemail.com.INVALID>
> wrote:
> >
> > Hi,
> >
> > While investigating some weirdness in custom Sling Model Injectors I came
> > across, that a SlingModel triggerd the invocation of the prepare() method
> > of a totally unrelated Injector, which implements the ValuePreparer
> > interface.
> >
> > I found that if no injector is given in the annotation [1] all available
> > injectors are taken [2], iterated and the ValuePreparer invoked (if it's
> > implemented by the Injector).
> >
> > That means that in case no specific injector is given, all injectors are
> > tried, which can be costly in terms of time. What I miss (and what I
> think
> > should be possible) is to reduce this list based on the type information.
> > Because it does not make sense to query an Injector which can only
> return a
> > String if an injection for the type SlingHttpServletRequest should be
> done.
> > Would such an optimization be possible?
> >
> > I don't have benchmark data yet how much time is spent in the process of
> > injecting all required fields, but before I invest more time here, I
> wanted
> > to understand if this is feasible at all.
> >
> > Thanks,
> > Jörg
> >
> >
> > [1]
> >
> https://github.com/apache/sling-org-apache-sling-models-impl/blob/ed1bd8d8a2fc01e13a10ba1c06122530c4d590b1/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L574
> >
> > [2]
> >
> https://github.com/apache/sling-org-apache-sling-models-impl/blob/ed1bd8d8a2fc01e13a10ba1c06122530c4d590b1/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L570
> >
> >
> >
> >
> > --
> > Cheers,
> > Jörg Hoh,
> >
> > https://cqdump.joerghoh.de
> > Twitter: @joerghoh
>
>

-- 
Cheers,
Jörg Hoh,

https://cqdump.joerghoh.de
Twitter: @joerghoh

Re: Sling Model AdapterFactory - iterating all injectors?

Posted by Konrad Windszus <ko...@gmx.de>.
Hi,
Shouldn’t we instead just more prominently promote injector-specific annotations (https://sling.apache.org/documentation/bundles/models.html#injector-specific-annotations <https://sling.apache.org/documentation/bundles/models.html#injector-specific-annotations>)?
IMHO this should be fixed in the model and we shouldn’t try to be smarter with generic injections.

Konrad

> On 22. Jun 2022, at 13:43, Jörg Hoh <jh...@googlemail.com.INVALID> wrote:
> 
> Hi,
> 
> While investigating some weirdness in custom Sling Model Injectors I came
> across, that a SlingModel triggerd the invocation of the prepare() method
> of a totally unrelated Injector, which implements the ValuePreparer
> interface.
> 
> I found that if no injector is given in the annotation [1] all available
> injectors are taken [2], iterated and the ValuePreparer invoked (if it's
> implemented by the Injector).
> 
> That means that in case no specific injector is given, all injectors are
> tried, which can be costly in terms of time. What I miss (and what I think
> should be possible) is to reduce this list based on the type information.
> Because it does not make sense to query an Injector which can only return a
> String if an injection for the type SlingHttpServletRequest should be done.
> Would such an optimization be possible?
> 
> I don't have benchmark data yet how much time is spent in the process of
> injecting all required fields, but before I invest more time here, I wanted
> to understand if this is feasible at all.
> 
> Thanks,
> Jörg
> 
> 
> [1]
> https://github.com/apache/sling-org-apache-sling-models-impl/blob/ed1bd8d8a2fc01e13a10ba1c06122530c4d590b1/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L574
> 
> [2]
> https://github.com/apache/sling-org-apache-sling-models-impl/blob/ed1bd8d8a2fc01e13a10ba1c06122530c4d590b1/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L570
> 
> 
> 
> 
> -- 
> Cheers,
> Jörg Hoh,
> 
> https://cqdump.joerghoh.de
> Twitter: @joerghoh