jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [941-ReduceELCalls] DISCUSSION

From: manfred riem <manfred.riem_at_oracle.com>
Date: Wed, 21 Jan 2015 10:42:57 -0600

Hi all,

After thinking about this one a little more I think we can solve this
problem nicer
since we now are moving more towards CDI.

As noted at https://struberg.wordpress.com/tag/cdi/ we could have the
CDI layer
cache the value and achieve a speed up that way.

Thoughts?
Manfred

On 1/15/15, 8:33 AM, arjan tijms wrote:
> Hi there,
>
> On Thu, Jan 15, 2015 at 11:11 AM, Hanspeter<hampidu_at_gmail.com> wrote:
>> Hi all.
>> A good thought this cache annotation, however, the called method to return
>> the rendered boolean has no knowledge if the component is inside an
>> iterating component. Cachong on phase level without respect to iteration
>> will cause problems.
> Yes, good point. There's definitely some more thinking needed there.
>
> Maybe the iterating component can reset the scoped cache (no need to
> go up or down the children's chain), or maybe the cache's
> (auto-generated) cache key can include the (virtual) component ID.
>
> As said, just thinking out loud here.
>
>
>> Also annotation lookup might cause more effort than simply cache rendered
>> eval result within the encode* cycle.
> Possibly yes. If the component itself does some caching and stores
> this in a simple variable, it's almost always going to be faster and
> as a bonus simpler and more automatic for the user as well.
>
> The idea about explicit caching tailored for JSF phases was just
> thrown out there in case this automatic caching would appear to be too
> invasive and the default has to be more conservative. It's a bit of a
> backup plan thus.
>
> One thing that could limit the cost of annotation lookup is if this
> would only be done once when the ValueExpression is created. Suppose
> we would create a CachingValueExpression when the @Cached annotation
> is present, and this CachingValueExpression would do the caching
> (possibly using a scoped cache).
>
> Components which would not care about caching can then just obtain the
> value from the VE as they always do, while components that do care can
> do an instanceof check, and if needed call methods on
> CachingValueExpression to bypass the cache, reset it, etc.
>
> Kind regards,
> Arjan
>
>
>> Regards
>> Hanspeter
>>
>> Am 14.01.2015 23:59 schrieb "arjan tijms"<arjan.tijms_at_gmail.com>:
>>
>>> Hi,
>>>
>>> On Tuesday, January 13, 2015, Leonardo Uribe<leonardo.uribe_at_irian.at>
>>> wrote:
>>>> Yes, the strategy done in MyFaces is far more simple, but since this is
>>>> considered an implementation detail, how it works falls out of the scope of
>>>> the discussion of the list.
>>> I for one would be rather interested in the details regarding this far
>>> more simple solution. Would it really not be a possible candidate for
>>> standardization?
>>>
>>>
>>>> The thing you need to consider in this problem is EL evaluation is not
>>>> as slow as you can imagine, compared with multiple hashmap.get() calls. On
>>>> the other side isRendered() method can be called multiple times and most of
>>>> them do not do any EL evaluation. If you have a component tree with multiple
>>>> component and some of them has an EL on isRendered(), you will not gain
>>>> anything if you add extra hashmap.get() calls on every isRendered() call
>>> I generally agree with the statement that we should be careful that
>>> any kind of cache lookup does not end up being roughly as expensive as
>>> the thing we're trying to cache, let alone be more expensive.
>>>
>>> That said, caching the isRendered outcome does prevent users getting
>>> surprised about why their bean is called multiple times, and lessens
>>> the impact of doing any kind of expensive computation inside the
>>> getter (which users of course shouldn't do, but especially beginners
>>> are often tempted to do anyway).
>>>
>>> Just thinking out loud, but a potential additional solution could be
>>> to put the caching behavior more in the hands of the user, for
>>> instance with explicit cached EL expressions, or JCache like
>>> annotations.
>>>
>>> Explicit cached value expressions are a bit like c:set or OmiFaces'
>>> o:cacheValue (http://showcase.omnifaces.org/components/cache), but
>>> would have more control over the phases. E.g.
>>>
>>> <o:cache key="foo" scope="request" phase="renderResponse">
>>> <o:cacheValue name="isRendered"
>>> value="#{someBean.something.isRendered}" />
>>> </o:cache>
>>>
>>> ...
>>>
>>> <h:panelGroup rendered="#{isRendered}">
>>> ...
>>>
>>> Or with a JCache-like annotation:
>>>
>>> @Cached(facesPhase="renderResponse")
>>> public boolean isRendered() {
>>> return ...
>>> }
>>>
>>> Yet another option with EL 3 lambdas:
>>>
>>> <h:panelGroup rendered="#{fn:cached( () ->
>>> someBean.something.isRendered, 'renderResponse') }">
>>> ...
>>>
>>> Such constructs could be used together with the component caching as
>>> mentioned in this thread; component caching could then be relatively
>>> conservative, while for more elaborate caching an explicit user
>>> controlled construct could be used.
>>>
>>> As said, just thinking out loud here ;)
>>>
>>> Kind regards,
>>> Arjan Tijms
>>>
>>>
>>>
>>>> , just to try to gain something on the components that use it (which can
>>>> be just a couple of them).
>>>>
>>>> regards,
>>>>
>>>> Leonardo Uribe
>>>>
>>>> 2015-01-12 15:55 GMT-05:00 manfred riem<manfred.riem_at_oracle.com>:
>>>>> Since you are telling me there are other tested strategies that are far
>>>>> more simple can you elaborate?
>>>>>
>>>>> Manfred
>>>>>
>>>>>
>>>>> On 1/12/15, 2:53 PM, Leonardo Uribe wrote:
>>>>>
>>>>> Hi Manfred
>>>>>
>>>>> Well, you can make it work, but all that logic seems overkill to me. It
>>>>> could work, but I doubt about its convenience against other tested
>>>>> strategies for the same problem that are far more simple.
>>>>>
>>>>> regards,
>>>>>
>>>>> Leonardo Uribe
>>>>>
>>>>> 2015-01-12 15:07 GMT-05:00 manfred riem<manfred.riem_at_oracle.com>:
>>>>>> Hi Leonardo,
>>>>>>
>>>>>> I disagree, as when the rendering starts in each encodeBegin it can
>>>>>> get the elCacheable transientHelper attribute from the parent and then set
>>>>>> is own elCacheable transientHelper attribute. And if you cannot find the
>>>>>> elCacheable transientHelper attribute on the parent you err on the side of
>>>>>> assuming it is not EL cacheable. And voila no going up many levels.
>>>>>>
>>>>>> Manfred
>>>>>>
>>>>>>
>>>>>> On 1/12/15, 12:26 PM, Leonardo Uribe wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> It could be one, it could be many. The problem is the component
>>>>>> doesn't know how many levels up should be checked to decide if there is an
>>>>>> iteration, so it will be forced to go all levels to the top, over and over.
>>>>>> There is no contextual information about the current iteration. I think it
>>>>>> is quite easy to find examples where it will fail.
>>>>>>
>>>>>> regards,
>>>>>>
>>>>>> Leonardo Uribe
>>>>>>
>>>>>> 2015-01-12 13:08 GMT-05:00 manfred riem<manfred.riem_at_oracle.com>:
>>>>>>> How so? It is just calling one component up?
>>>>>>>
>>>>>>> Manfred
>>>>>>>
>>>>>>>
>>>>>>> On 1/12/15, 12:06 PM, Leonardo Uribe wrote:
>>>>>>>
>>>>>>> Hi Manfred
>>>>>>>
>>>>>>> MR>> The encodeBegin method has access to the parent component
>>>>>>>
>>>>>>> The problem is do that will increase the algorithm complexity. We
>>>>>>> need to
>>>>>>> keep it on lineal complexity (O(n)), otherwise the optimization will
>>>>>>> not
>>>>>>> be effective.
>>>>>>>
>>>>>>> regards,
>>>>>>>
>>>>>>> Leonardo Uribe
>>>>>>>
>>>>>>> 2015-01-12 12:44 GMT-05:00 manfred riem<manfred.riem_at_oracle.com>:
>>>>>>>> Hi Leonardo,
>>>>>>>>
>>>>>>>> The encodeBegin method has access to the parent component, so it can
>>>>>>>> determine the parent ElCacheable from it. If the parent is not an
>>>>>>>> ELCacheable instance the component itself cannot be either, otherwise it can
>>>>>>>> look its getter.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Manfred
>>>>>>>>
>>>>>>>>
>>>>>>>> On 1/12/15, 11:40 AM, Leonardo Uribe wrote:
>>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> I don't think an interface could work, because it is the parent, not
>>>>>>>> the child who defines the iteration strategy. It is a fact that we reuse the
>>>>>>>> same component instances for different rows (UIData).
>>>>>>>>
>>>>>>>> regards,
>>>>>>>>
>>>>>>>> Leonardo Uribe
>>>>>>>>
>>>>>>>> 2015-01-12 9:13 GMT-05:00 manfred riem<manfred.riem_at_oracle.com>:
>>>>>>>>> Hi Hanspeter,
>>>>>>>>>
>>>>>>>>> I am trying to formalize it for the RenderPhase only, which would
>>>>>>>>> mean stating something like the following:
>>>>>>>>>
>>>>>>>>> 1. ElCacheable would be defined by an interface
>>>>>>>>> 2. ElCacheable would operate only in RENDER_RESPONSE
>>>>>>>>> a. Which means a component would not be allowed to change the
>>>>>>>>> value of an EL expression
>>>>>>>>> b. For an iterating component it would not cache and thus not
>>>>>>>>> implement ElCacheable
>>>>>>>>> c. For components not implementing the ElCacheable interface it
>>>>>>>>> would operate as if it was not cacheable.
>>>>>>>>> 3. Add a getter / setter that stores elCacheable on the UIComponent
>>>>>>>>>
>>>>>>>>> Am I missing something here?
>>>>>>>>>
>>>>>>>>> Thoughts?
>>>>>>>>> Manfred
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 1/9/15, 5:18 PM, Hanspeter wrote:
>>>>>>>>>
>>>>>>>>> Hi Experts.
>>>>>>>>>
>>>>>>>>> I think this is worth pushing +1.
>>>>>>>>>
>>>>>>>>> The multiple rendered attribute evaluation bothers most in the
>>>>>>>>> render phase because isRendered() is called by every
>>>>>>>>> UIComponent(Base).encode* method. So maybe a first step would be to improve
>>>>>>>>> the encode* methods such that for an encodeAll() cycle the rendered
>>>>>>>>> attribute is cached. That is possible with a few lines of code - I happily
>>>>>>>>> provide the changes.
>>>>>>>>>
>>>>>>>>> E.g. encodeAll() and/or encodeBegin() could enable renderedCaching,
>>>>>>>>> isRendered() stores the rendered result on a component private Boolean, and
>>>>>>>>> in encodeEnd() rendered caching is disabled and reset. That way without
>>>>>>>>> additional effort rendered is reset for the next iteration. But still this
>>>>>>>>> would reduce evaluate times by factor 3 to 4 per component encoding.
>>>>>>>>>
>>>>>>>>> I'm not sure whether rendered caching on a broader scope makes
>>>>>>>>> sense, e.g. for phases 2-5. In csJSF component library we did some rendered
>>>>>>>>> caching, but had to reset the cache once per phase and within iterating
>>>>>>>>> components as well. And I think rendered is rarely evaluated more often as
>>>>>>>>> it was reset in these execution phases.
>>>>>>>>>
>>>>>>>>> Best regards
>>>>>>>>> Hanspeter
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2015-01-09 19:12 GMT+01:00 Bauke Scholtz<balusc_at_gmail.com>:
>>>>>>>>>> +1
>>>>>>>>>>
>>>>>>>>>> Ideally, the UIComponent#isRendered() should cache the value on a
>>>>>>>>>> per-phase and per-iteration basis. It would be awesome too if iteration
>>>>>>>>>> components implement a common interface or abstract class, which should also
>>>>>>>>>> simplify a lot of other things related to a.o. setting the currently
>>>>>>>>>> iterated item and state saving.
>>>>>>>>>>
>>>>>>>>>> Cheers, B
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Fri, Jan 9, 2015 at 7:00 PM, manfred riem
>>>>>>>>>> <manfred.riem_at_oracle.com> wrote:
>>>>>>>>>>> Whoops, sending to right list. Please ignore it on the other.
>>>>>>>>>>>
>>>>>>>>>>> -------- Original Message --------
>>>>>>>>>>> Subject: [jsr344-experts] [941-ReduceELCalls] DISCUSSION
>>>>>>>>>>> Date: Fri, 09 Jan 2015 11:51:14 -0600
>>>>>>>>>>> From: manfred riem<manfred.riem_at_oracle.com>
>>>>>>>>>>> Reply-To: jsr344-experts_at_javaserverfaces-spec-public.java.net
>>>>>>>>>>> Organization: Oracle Corporation
>>>>>>>>>>> To: jsr344-experts_at_javaserverfaces-spec-public.java.net
>>>>>>>>>>> CC: Max Starets<max.starets_at_oracle.com>, Andy Schwartz
>>>>>>>>>>> <andy.schwartz_at_oracle.com>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> As we all know the number of times an EL expression is evaluated
>>>>>>>>>>> during render is higher than it could be.
>>>>>>>>>>>
>>>>>>>>>>> To see if there is a potential to reduce that number I want to
>>>>>>>>>>> ask the most important question first as it determines whether or not this
>>>>>>>>>>> issue is even worth pursuing?
>>>>>>>>>>>
>>>>>>>>>>> Is it safe to assume that during rendering and not as a child of
>>>>>>>>>>> an iterating component the value of the EL expression will stay stable?
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Manfred
>>>>>>>>>>>