myfaces-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Wesley Hales" <wesleyha...@gmail.com>
Subject Re: Multiple calls on isRendered - Performance issue
Date Wed, 09 May 2007 15:13:07 GMT
Yeah, not sure how to approach this one... Craig or any comitters have any
ideas?
Thanks for the help Guys....Anybody who cares about performance will
definitely be able to use this enhancement.

On 5/9/07, Andrew Robinson <andrew.rw.robinson@gmail.com> wrote:
>
> Sorry for the 2 emails,
>
> I don't think you are allowed to add methods to the javax.faces API
> classes. That will break TCK compatibility right? If that is true,
> MyFaces would no longer be an official JSF provider.
>
> On 5/9/07, Andrew Robinson <andrew.rw.robinson@gmail.com> wrote:
> > You may want to submit a comment for the JSF specification on changing
> > this behavior.
> >
> > On 5/9/07, Wesley Hales <wesleyhales@gmail.com> wrote:
> > > So the architect I am working with (Chris Kulinski) came up with the
> > > following patch to 1.1.5...
> > >
> > > Index: src/main/java/javax/faces/component/UIComponent.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIComponent.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIComponent.java
> > > (working copy)
> > > @@ -59,6 +59,8 @@
> > >
> > >      public abstract boolean isRendered();
> > >
> > > +    public abstract boolean isRenderedCached();
> > > +
> > >      public abstract void setRendered(boolean rendered);
> > >
> > >      public abstract java.lang.String getRendererType();
> > > Index: src/main/java/javax/faces/component/UIMessages.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIMessages.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIMessages.java
> > > (working copy)
> > > @@ -28,7 +28,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIMessages
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      //------------------ GENERATED CODE BEGIN (do not modify!)
> > > --------------------
> > >
> > > Index: src/main/java/javax/faces/component/UIData.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIData.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIData.java
> > > (working copy)
> > > @@ -126,7 +126,7 @@
> > >   * @author Manfred Geiler (latest modification by $Author$)
> > >   * @version $Revision$ $Date$
> > >   */
> > > -public class UIData extends UIComponentBase implements
> NamingContainer
> > > +public class UIData extends UIComponentBaseCached implements
> > > NamingContainer
> > >  {
> > >      private static final int STATE_SIZE = 5;
> > >      private static final int SUPER_STATE_INDEX = 0;
> > > @@ -577,7 +577,7 @@
> > >      {
> > >          if (context == null)
> > >              throw new NullPointerException("context");
> > > -        if (!isRendered())
> > > +        if (!isRenderedCached())
> > >              return;
> > >          setRowIndex(-1);
> > >          processFacets(context, PROCESS_DECODES);
> > > @@ -599,7 +599,7 @@
> > >      {
> > >          if (context == null)
> > >              throw new NullPointerException("context");
> > > -        if (!isRendered())
> > > +        if (!isRenderedCached())
> > >              return;
> > >          setRowIndex(-1);
> > >          processFacets(context, PROCESS_VALIDATORS);
> > > @@ -618,7 +618,7 @@
> > >      {
> > >          if (context == null)
> > >              throw new NullPointerException("context");
> > > -        if (!isRendered())
> > > +        if (!isRenderedCached())
> > >              return;
> > >          setRowIndex(-1);
> > >          processFacets(context, PROCESS_UPDATES);
> > > @@ -656,7 +656,7 @@
> > >              UIComponent child = (UIComponent) childIter.next();
> > >              if (child instanceof UIColumn)
> > >               {
> > > -                if (!child.isRendered())
> > > +                if (!child.isRenderedCached())
> > >                  {
> > >                      //Column is not visible
> > >                      continue;
> > > @@ -705,7 +705,7 @@
> > >                  UIComponent child = (UIComponent) it.next();
> > >                  if (child instanceof UIColumn)
> > >                  {
> > > -                    if (!child.isRendered())
> > > +                    if (!child.isRenderedCached())
> > >                      {
> > >                          //Column is not visible
> > >                          continue;
> > > Index: src/main/java/javax/faces/component/UIParameter.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIParameter.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIParameter.java
> > > (working copy)
> > > @@ -28,7 +28,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIParameter
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      //------------------ GENERATED CODE BEGIN (do not modify!)
> > > --------------------
> > >
> > > Index: src/main/java/javax/faces/component/UIMessage.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIMessage.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIMessage.java
> > > (working copy)
> > > @@ -30,7 +30,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIMessage
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      //------------------ GENERATED CODE BEGIN (do not modify!)
> > > --------------------
> > >
> > > Index: src/main/java/javax/faces/component/UIForm.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIForm.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIForm.java
> > > (working copy)
> > > @@ -28,7 +28,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIForm
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >          implements NamingContainer
> > >  {
> > >      //private static final Log log = LogFactory.getLog(UIForm.class);
> > > Index: src/main/java/javax/faces/component/UIGraphic.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIGraphic.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIGraphic.java
> > > (working copy)
> > > @@ -28,7 +28,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIGraphic
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      private static final String URL_PROPERTY = "url";
> > >      private static final String VALUE_PROPERTY = "value";
> > > Index: src/main/java/javax/faces/component/UICommand.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UICommand.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UICommand.java
> > > (working copy)
> > > @@ -31,7 +31,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >   public class UICommand
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >          implements ActionSource
> > >  {
> > >      private MethodBinding _action = null;
> > > Index: src/main/java/javax/faces/component/UIViewRoot.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIViewRoot.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIViewRoot.java
> > > (working copy)
> > > @@ -38,7 +38,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIViewRoot
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      public static final String UNIQUE_ID_PREFIX = "_id";
> > >
> > > Index: src/main/java/javax/faces/component/UIColumn.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIColumn.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIColumn.java
> > > (working copy)
> > > @@ -27,7 +27,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIColumn
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      private static final String FOOTER_FACET_NAME = "footer";
> > >      private static final String HEADER_FACET_NAME = "header";
> > > Index:
> > > src/main/java/javax/faces/component/UIComponentBase.java
> > > ===================================================================
> > > ---
> > > src/main/java/javax/faces/component/UIComponentBase.java
> > >    (revision 536301)
> > > +++
> > > src/main/java/javax/faces/component/UIComponentBase.java
> > >    (working copy)
> > > @@ -508,7 +508,7 @@
> > >              throws IOException
> > >      {
> > >          if (context == null) throw new
> > > NullPointerException("context");
> > > -        if (!isRendered()) return;
> > > +        if (!isRenderedCached()) return;
> > >          Renderer renderer = getRenderer(context);
> > >          if (renderer != null)
> > >          {
> > > @@ -520,7 +520,7 @@
> > >              throws IOException
> > >      {
> > >          if (context == null) throw new
> > > NullPointerException("context");
> > > -        if (!isRendered()) return;
> > > +        if (!isRenderedCached()) return;
> > >          Renderer renderer = getRenderer(context);
> > >          if (renderer != null)
> > >          {
> > > @@ -532,7 +532,7 @@
> > >              throws IOException
> > >      {
> > >          if (context == null) throw new
> > > NullPointerException("context");
> > > -        if (!isRendered()) return;
> > > +        if (!isRenderedCached()) return;
> > >          Renderer renderer = getRenderer(context);
> > >          if (renderer != null)
> > >          {
> > > @@ -598,7 +598,7 @@
> > >      public void processDecodes(FacesContext context)
> > >      {
> > >          if (context == null) throw new
> > > NullPointerException("context");
> > > -                if (!isRendered()) return;
> > > +                if (!isRenderedCached()) return;
> > >          for (Iterator it = getFacetsAndChildren(); it.hasNext(); )
> > >          {
> > >              UIComponent childOrFacet = (UIComponent)it.next();
> > > @@ -619,7 +619,7 @@
> > >      public void processValidators(FacesContext context)
> > >      {
> > >          if (context == null) throw new
> > > NullPointerException("context");
> > > -        if (!isRendered()) return;
> > > +        if (!isRenderedCached()) return;
> > >
> > >          for (Iterator it = getFacetsAndChildren(); it.hasNext(); )
> > >          {
> > > @@ -640,7 +640,7 @@
> > >      public void processUpdates(FacesContext context)
> > >      {
> > >          if (context == null) throw new
> > > NullPointerException("context");
> > > -        if (!isRendered()) return;
> > > +        if (!isRenderedCached()) return;
> > >
> > >          for (Iterator it = getFacetsAndChildren(); it.hasNext(); )
> > >          {
> > > @@ -1063,6 +1063,7 @@
> > >
> > >      private Boolean _rendered = null;
> > >      private String _rendererType = null;
> > > +    private Boolean _renderedCached = null;
> > >
> > >
> > >
> > > @@ -1072,11 +1073,29 @@
> > >      }
> > >
> > >      public boolean isRendered()
> > > +    {
> > > +        if (_rendered != null) return _rendered.booleanValue();
> > > +        ValueBinding vb = getValueBinding("rendered");
> > > +        // AUTOTRADER: Cache the rendered value so its not computed
> > > multiple times.
> > > +        _renderedCached = vb != null ?
> > > (Boolean)vb.getValue(getFacesContext()) : null;
> > > +        return _renderedCached != null ?
> _renderedCached.booleanValue() :
> > > DEFAULT_RENDERED;
> > > +    }
> > > +
> > > +    public boolean isRenderedCached()
> > >      {
> > >          if (_rendered != null) return _rendered.booleanValue();
> > > +
> > > +        // AUTOTRADER: Cache the rendered value so its not computed
> > > multiple times.
> > > +        if (_renderedCached != null) {
> > > +            //System.out.println("using CACHED value for:"+getId()+"
> > > "+this);
> > > +            return _renderedCached.booleanValue();
> > > +        }
> > > +
> > >          ValueBinding vb = getValueBinding("rendered");
> > > -        Boolean v = vb != null ?
> > > (Boolean)vb.getValue(getFacesContext()) : null;
> > > -        return v != null ? v.booleanValue() : DEFAULT_RENDERED;
> > > +        // AUTOTRADER: Cache the rendered value so its not computed
> > > multiple times.
> > > +        _renderedCached = vb != null ?
> > > (Boolean)vb.getValue(getFacesContext()) : null;
> > > +        //System.out.println("Computing NO CACHE value
> for:"+getId()+"
> > > "+this);
> > > +        return _renderedCached != null ?
> _renderedCached.booleanValue() :
> > > DEFAULT_RENDERED;
> > >      }
> > >
> > >      public void setRendererType(String rendererType)
> > > Index: src/main/java/javax/faces/component/UIPanel.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIPanel.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIPanel.java
> > > (working copy)
> > > @@ -27,7 +27,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIPanel
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >  {
> > >      //------------------ GENERATED CODE BEGIN (do not modify!)
> > > --------------------
> > >
> > > Index: src/main/java/javax/faces/component/UIOutput.java
> > > ===================================================================
> > > --- src/main/java/javax/faces/component/UIOutput.java
> > > (revision 536301)
> > > +++ src/main/java/javax/faces/component/UIOutput.java
> > > (working copy)
> > > @@ -29,7 +29,7 @@
> > >   * @version $Revision$ $Date$
> > >   */
> > >  public class UIOutput
> > > -        extends UIComponentBase
> > > +        extends UIComponentBaseCached
> > >          implements ValueHolder
> > >  {
> > >      public Object getLocalValue()
> > >
> ---------------------------------------------------------------------------------
> > >
> > > and here is the UIComponentBaseCached class:
> > >
> > > package javax.faces.component;
> > >
> > > public abstract class UIComponentBaseCached extends UIComponentBase {
> > >
> > >     public boolean isRendered()
> > >     {
> > >         return isRenderedCached();
> > >     }
> > > }
> > >
> > >
> > > So far so good, I will post the results if anything stops working.
> > >
> > > Wesley
> > >
> > >
> > >
> > >
> > >  On 5/9/07, Wesley Hales <wesleyhales@gmail.com> wrote:
> > > > I am trying to address this at a framework level (i.e. I'm not
> rewriting
> > > the renderer for each component that calls an EL method)
> > > >
> > > >
> > > >
> > > > On 5/9/07, Adam Winer <awiner@gmail.com> wrote:
> > > > > Technically, rendered is supposed to be:
> > > > > - Constant across
> > > encodeBegin()/encodeChildren()/encodeEnd()
> > > > > - Constant across processDecodes()/processValidators()/
> > > > >    processUpdateModel().
> > > > >
> > > > > Unfortunately, the latter doesn't help - since a component inside
> > > > > a table would have processDecodes() called repeatedly,
> > > > > then processValidators() repeatedly, etc. - and "rendered"
> > > > > could be different from one row to the next.
> > > > >
> > > > > But the former should be fine - if you always fetch and
> > > > > store the value in encodeBegin() (whether or not encodeBegin()
> > > > > has previously been called), then use that in encodeChildren()
> > > > > and encodeEnd(), you should be fine.
> > > > >
> > > > > -- Adam
> > > > >
> > > > >
> > > > > On 5/8/07, Simon Kitching < simon.kitching@rhe.co.nz> wrote:
> > > > > > Andrew, are you just stating what currently happens or are you
> saying
> > > > > > that there is a *reason* for evaluating "rendered" in
> encodeBegin,
> > > > > > encodeChildren and encodeEnd?
> > > > > >
> > > > > > I can't initially see any reason why it would need to be
> evaluated
> > > more
> > > > > > than once.
> > > > > >
> > > > > > UIComponentBase does indeed call isRendered in encodeBegin,
> > > > > > encodeChildren and encodeEnd. It also implements isRendered
as
> an
> > > > > > evaluation of the EL expression (myfaces 1.1.3 implementation).
> > > > > >
> > > > > > But As Wesley asks, why would it ever make sense for rendered
to
> be
> > > true
> > > > > > in encodeBegin, but false in encodeEnd? I cannot see any way
> that
> > > would
> > > > > > be useful, so why not just compute it once then cache it
> somewhere for
> > > > > > the rest of that render cycle (eg in a "transient" member of
the
> > > > > > component)? This would be a significant performance boost..
> > > > > >
> > > > > > Wesley, your proposed modification which stores the rendered
> state
> > > into
> > > > > > _rendered is not good because setting _rendered will permanently
> > > > > > override the rendered EL expression, not just for the current
> render
> > > > > > cycle but for the lifetime of that component. What is needed
is
> to
> > > > > > figure out when the first call to isRendered is done *during
the
> > > render
> > > > > > cycle* and then cache that until rendering finishes. Note that
> > > > > > isRendered is also called during postback processing, and it's
> > > perfectly
> > > > > > reasonable to change this value between postback and render
so
> caching
> > > > > > really should only be done between encodeBegin() and
> encodeEnd(). So
> > > > > > implementing this optimisation is a little tricky - but not
> impossible
> > > > > > I'm sure.
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Simon
> > > > > >
> > > > > > Andrew Robinson wrote:
> > > > > > > At the very least rendered is called during encodeBegin,
> > > > > > > encodeChildren, and encodeEnd.
> > > > > > >
> > > > > > > On 5/8/07, Wesley Hales < wesleyhales@gmail.com>
wrote:
> > > > > > >> Hello - Why do we continually call isRendered after
> encodeBegin()?
> > > > > > >> Once the
> > > > > > >> begin tag is written out, it shouldn't matter what
the body
> and end
> > > > > > >> rendered
> > > > > > >> states are.
> > > > > > >>
> > > > > > >> Facelets 1.1.11
> > > > > > >>  Myfaces 1.1.5 & 1.2
> > > > > > >>
> > > > > > >> So if I have  <t:div rendered="#{ MyBean.alerts
> 0}"...
> > > > > > >> The #{MyBean.alerts method is called 5 times! and it
is like
> this
> > > for
> > > > > > >> every
> > > > > > >> EL eval on the page. I do know that EL can only bind
to the
> > > > > > >> FaceletsContext
> > > > > > >> and no other scopes... So are there any other options
than
> what I
> > > have
> > > > > > >> listed below?
> > > > > > >>
> > > > > > >> 1. Tried to modify MyFaces src in UIComponentBase:
> > > > > > >> public boolean isRendered()
> > > > > > >>      {
> > > > > > >>            if(_rendered == null){
> > > > > > >>
> > > > > > >>            // our modification! Only compute the rendered
> value
> > > once, and
> > > > > > >> cache for the rest of the lifecycle.
> > > > > > >>            Boolean rend = getExpressionValue("rendered",
> _rendered,
> > > > > > >> DEFAULT_RENDERED);
> > > > > > >>            _rendered = rend.booleanValue();
> > > > > > >>            }
> > > > > > >>          return _rendered;
> > > > > > >>      }
> > > > > > >> This didn't work :( - Something happend to A4J and
we had no
> ideas
> > > > > > >> what the
> > > > > > >> implication would be on all components.
> > > > > > >>
> > > > > > >> 2. Tried using Facelets <ui:param and <c:set
to store the EL
> in a
> > > page
> > > > > > >> scoped variable, then have the variable evaluated in
the
> rendered
> > > > > > >> attribute.
> > > > > > >> This didn't work either because it is on FacesContext.
> > > > > > >>
> > > > > > >> So this may just be me not fully understanding  what
JSF does
> > > behind the
> > > > > > >> scenes of component rendering, or some people say that
 the
> spec is
> > > > > > >> screwed
> > > > > > >> up when it comes to this. I'm sure some of the pros
can help
> me out
> > > > > > >> here :).
> > > > > > >>
> > > > > > >> Thx,
> > > > > > >> Wesley Hales
> > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> >
>

Mime
View raw message