MyFaces Core
  1. MyFaces Core
  2. MYFACES-3237

[PERF] Renderers for components like h:outputText and others do many unecessary getAttributes().get() calls

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.8, 2.1.2
    • Component/s: JSR-314
    • Labels:
      None

      Description

      Doing some performance tests with Gerhard, we notice renderers like the one used for h:outputText does a lot of calls to getAttributes().get().

      The problem is most of this calls just return null, wasting time and resources. For example, usually h:outputText uses value, style and styleClass. Think about the most basic use case where you need to show some data with just h:dataTable and h:outputText. Per each h:outputText, 6 calls to getAttributes().get() for passthrough attributes, most of them completely unnecessary. The same is true for other components.

      We can reduce the number of calls if we have something that keep track of the properties already set. Obviously this will be a myfaces internal, but I think it is worth to do it at least for the most used components like h:outputText, h:outputLabel, h:outputFormat, h:outputScript and h:outputStylesheet. Note do this can make renderers harder to maintain, because some extra code should be added.

      We already have some interfaces in javax.faces.component.html package like

      _StyleProperties
      _UniversalProperties
      _TitleProperty
      _EscapeProperty
      _DisabledClassEnabledClassProperties
      _DisabledReadonlyProperties
      _AccesskeyProperty
      _AltProperty
      _ChangeSelectProperties
      _EventProperties
      _FocusBlurProperties
      _LabelProperty
      _LinkProperties
      _MessageProperties
      _TabindexProperty

      The idea could be focus only on these set of properties, and let the others as is.

      Long time ago, when I reviewed trinidad code for implement MyFaces Core Partial State Saving, I notice the class org.apache.myfaces.trinidad.bean.PropertyKey has an algorithm that assign a number to each property. Maybe we can assign a number to each property and store a flag on attribute map when it is set, and then retrieve this value from the renderer and only check the ones that needs to be rendered.

        Issue Links

          Activity

          Leonardo Uribe made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Matt Benson made changes -
          Link This issue is blocked by MYFACES-3256 [ MYFACES-3256 ]
          Leonardo Uribe made changes -
          Field Original Value New Value
          Status Open [ 1 ] Resolved [ 5 ]
          Assignee Leonardo Uribe [ lu4242 ]
          Fix Version/s 2.0.8 [ 12316514 ]
          Fix Version/s 2.1.2 [ 12316512 ]
          Resolution Fixed [ 1 ]
          Hide
          Leonardo Uribe added a comment -

          The patch commited only was applied to the most important cases. It is still possible to add it to components like h:selectXXX, but for now it is ok. The idea was just create a class CommonPropertyConstants and some methods that can be called modifying the default template for create components. A new method was added on HtmlRenderer called isCommonPropertiesOptimizationEnabled() to enable/disable the optimization only to default renderers.

          Show
          Leonardo Uribe added a comment - The patch commited only was applied to the most important cases. It is still possible to add it to components like h:selectXXX, but for now it is ok. The idea was just create a class CommonPropertyConstants and some methods that can be called modifying the default template for create components. A new method was added on HtmlRenderer called isCommonPropertiesOptimizationEnabled() to enable/disable the optimization only to default renderers.
          Hide
          Martin Kočí added a comment -

          This is good idea. I noticed exactly the same problem (h:outputText in a big h:dataTable) a year ago and solved it with custom a:fastOutputText (supports only value and styleClass). Proposes solution is will help all users.

          Also this is useful for logging. Currently we output warnings like " ...has no attribute url, value, name or attribute resolves to null..." (MYFACES-3243 for example). Checking if property is set will help distinguish this situation.

          Show
          Martin Kočí added a comment - This is good idea. I noticed exactly the same problem (h:outputText in a big h:dataTable) a year ago and solved it with custom a:fastOutputText (supports only value and styleClass). Proposes solution is will help all users. Also this is useful for logging. Currently we output warnings like " ...has no attribute url, value, name or attribute resolves to null..." ( MYFACES-3243 for example). Checking if property is set will help distinguish this situation.
          Leonardo Uribe created issue -

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Leonardo Uribe
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development