Uploaded image for project: 'MyFaces Core'
  1. MyFaces Core
  2. MYFACES-3774

[perf] use facetName as a hint when try to find a component on refresh view algorithm

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2.0-beta
    • Component/s: None
    • Labels:
      None

      Description

      Checking some stuff I realized in ComponentTagHandlerDelegate.apply(...), the following instructions are done when the view requires refresh in render response phase:

      String facetName = this.getFacetName(ctx, parent);

      ....

      c = ComponentSupport.findChildByTagId(parent, id);

      ....

      if (facetName == null)

      { parent.getChildren().remove(c); }

      else

      { ComponentSupport.removeFacet(ctx, parent, c, facetName); }

      ....

      if (facetName == null)

      { parent.getChildren().add(c); }

      else

      { ComponentSupport.addFacet(ctx, parent, c, facetName); }

      The thing is if facetName == null the component will always be as a children. If facetName != null, the component will always be in the facet associated to that name. But the code in findChildByTagId(...) always look on the children first and then in all facets using an iterator, which is just overkill.

      It is better if we split findChildByTagId(...) into two variants (findChildInFacetByTagId and findChildInChildrenByTagId) and use the facetName as the condition and to get the right one if the component is in the facet. That will save 1 iterator instance per facet and a lot of unnecessary component.getAttributes().get(...) calls. But from the other side, myfaces algorithm is smart enough to avoid view refreshing at all, so the improvement will only be seen in some selected situations. Anyway, it is worth to do it for 2.2.x branch.

        Attachments

          Activity

            People

            • Assignee:
              lu4242 Leonardo Uribe
              Reporter:
              lu4242 Leonardo Uribe

              Dates

              • Created:
                Updated:
                Resolved:

                Issue deployment