MyFaces Core
  1. MyFaces Core
  2. MYFACES-3136

[perf] review UIComponentBase.getRendererType

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.0-SNAPSHOT
    • Fix Version/s: 2.0.13, 2.1.7
    • Component/s: General
    • Labels:
      None
    • Environment:
      myfaces core trunk

      Description

      1) method UIComponentBase.getRendererType is the most frequent, because all encodeBegin, getRendersChildren and encodeAll l try to locate renderer

      2) getRendererType uses StateHelper.eval - but can be renderType ValueExpression? If not, change StateHelper.eval to StateHelper.get

      3) should rendererType even be part of state saving? Each component I've ever seen has setRendererType("com.foo.renderer") in constructor and/or VDL calls setRendererType() after calling Application.createComponent(): If rendererType is not part of state saving, replace StateHelper with attribute

      4) Cache getRenderer(FacesContext context) result anyway (I'll create separate issue for this)

        Issue Links

          Activity

          Martin Kočí created issue -
          Hide
          Martin Kočí added a comment -

          Dev mailling list discussion: http://www.mail-archive.com/dev@myfaces.apache.org/msg53054.html

          Result: change UIComponent.rendererType to StateHelper().get, it cannot have ValueExpression.

          Show
          Martin Kočí added a comment - Dev mailling list discussion: http://www.mail-archive.com/dev@myfaces.apache.org/msg53054.html Result: change UIComponent.rendererType to StateHelper().get, it cannot have ValueExpression.
          Hide
          Martin Kočí added a comment -

          If no objections, I'll commit the change soon!

          Show
          Martin Kočí added a comment - If no objections, I'll commit the change soon!
          Martin Kočí made changes -
          Field Original Value New Value
          Attachment MYFACES-3136.patch [ 12479454 ]
          Hide
          Martin Kočí added a comment -

          patch committed to trunk svn rev. 1124213; checked trinidad, primefaces, richfaces and icefaces: they don't expect rendererType as valueExpression.

          Show
          Martin Kočí added a comment - patch committed to trunk svn rev. 1124213; checked trinidad, primefaces, richfaces and icefaces: they don't expect rendererType as valueExpression.
          Martin Kočí made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Assignee Martin Kočí [ markoc50 ]
          Fix Version/s 2.1.0-SNAPSHOT [ 12316325 ]
          Resolution Fixed [ 1 ]
          Leonardo Uribe made changes -
          Fix Version/s 2.1.0 [ 12315190 ]
          Fix Version/s 2.1.0-SNAPSHOT [ 12316325 ]
          Leonardo Uribe made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Leonardo Uribe added a comment -

          I checked this one again and even if the improvement works for all known jsf libraries, , according to JSF spec section 8 Rendering Model,this part is essential to implement "delegated implementation" pattern, so we can't do this optimization here. Instead, JSF developers could prevent this evaluation overriding this method directly on the created components, like trinidad does. In other words, it is valid to have a ValueExpression attached to "rendererType". Unfortunately I have to revert this change. Instead, we should minimize lookups to attribute map using other means, like directly on the renderers, as suggested on MYFACES-3237

          Show
          Leonardo Uribe added a comment - I checked this one again and even if the improvement works for all known jsf libraries, , according to JSF spec section 8 Rendering Model,this part is essential to implement "delegated implementation" pattern, so we can't do this optimization here. Instead, JSF developers could prevent this evaluation overriding this method directly on the created components, like trinidad does. In other words, it is valid to have a ValueExpression attached to "rendererType". Unfortunately I have to revert this change. Instead, we should minimize lookups to attribute map using other means, like directly on the renderers, as suggested on MYFACES-3237
          Leonardo Uribe made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Leonardo Uribe made changes -
          Status Reopened [ 4 ] Closed [ 6 ]
          Resolution Won't Fix [ 2 ]
          Leonardo Uribe made changes -
          Resolution Won't Fix [ 2 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Assignee Martin Kočí [ markoc50 ] Leonardo Uribe [ lu4242 ]
          Leonardo Uribe made changes -
          Link This issue is part of MYFACES-3488 [ MYFACES-3488 ]
          Leonardo Uribe made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Fix Version/s 2.0.13 [ 12319847 ]
          Fix Version/s 2.1.7 [ 12319845 ]
          Fix Version/s 2.1.0 [ 12315190 ]
          Resolution Fixed [ 1 ]
          Hide
          Leonardo Uribe added a comment -

          fixed as part of MYFACES-3488

          Show
          Leonardo Uribe added a comment - fixed as part of MYFACES-3488
          Leonardo Uribe made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Martin Kočí
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development