MyFaces Core
  1. MyFaces Core
  2. MYFACES-2737

Cache FacesContext on UIComponentBase instances

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 1.1.8, 1.2.9, 2.0.1
    • Component/s: JSR-314
    • Labels:
      None

      Description

      Right now, the implementation of UIComponentBase.getFacesContext() is this:

      @Override
      protected FacesContext getFacesContext()

      { return FacesContext.getCurrentInstance(); }

      I think it is possible to create a variable like this:

      private transient FacesContext _facesContext;

      and change the current implementation to:

      void setCachedFacesContext(FacesContext facesContext)

      { _facesContext = facesContext; }

      @Override
      protected FacesContext getFacesContext()
      {
      if (_facesContext == null)

      { return FacesContext.getCurrentInstance(); }

      else

      { return _facesContext; }

      }

      Then we do this on methods like processXXX, encodeXXX (not on encodeAll), visitTree and invokeOnComponent:

      @Override
      public void processDecodes(FacesContext context)
      {
      try

      { setCachedFacesContext(context); /*...... do what is required ........*/ }

      finally

      { popComponentFromEL(context); setCachedFacesContext(null); }

      In few words, set and release temporally the variable while those operations are executed. This change will reduce the amount of calls to FacesContext.getCurrentInstance() without side effects, because we are caching only on safe places and enclosing everything in a try finally block.

      If no objections I'll commit this code soon.

      1. MYFACES-2737-jsf20.patch
        32 kB
        Leonardo Uribe
      2. MYFACES-2737-jsf12.patch
        22 kB
        Leonardo Uribe
      3. MYFACES-2737-jsf11.patch
        12 kB
        Leonardo Uribe
      4. MYFACES-2737-1.patch
        31 kB
        Leonardo Uribe

        Activity

        Hide
        Martin Marinschek added a comment -

        Hi Leonardo,

        I definitely like this approach - no spec changes necessary. Way cool!

        It was that simple, and I just didn't see it . Neither did any of the cs-JSF team and EG members involved - just great.

        best regards,

        Martin

        Show
        Martin Marinschek added a comment - Hi Leonardo, I definitely like this approach - no spec changes necessary. Way cool! It was that simple, and I just didn't see it . Neither did any of the cs-JSF team and EG members involved - just great. best regards, Martin
        Hide
        Jan-Kees van Andel added a comment -

        I agree with Martin. It looks good.

        Normally I'm quite keen on concurrency bugs (like putting non-threadsafe classes in the session), but this approach where you manually keep the FacesContext instance request scoped, looks safe.

        How many FacesContext.getCurrentInstance calls do you expect to save by this approach?

        Show
        Jan-Kees van Andel added a comment - I agree with Martin. It looks good. Normally I'm quite keen on concurrency bugs (like putting non-threadsafe classes in the session), but this approach where you manually keep the FacesContext instance request scoped, looks safe. How many FacesContext.getCurrentInstance calls do you expect to save by this approach?
        Hide
        Leonardo Uribe added a comment -

        With this hack we are saving a lot of calls. There is one call to FacesContext.getCurrentInstance() per each ValueBinding/ValueExpression that it is rendered. I attached the final patch and committed it on all branches.

        Show
        Leonardo Uribe added a comment - With this hack we are saving a lot of calls. There is one call to FacesContext.getCurrentInstance() per each ValueBinding/ValueExpression that it is rendered. I attached the final patch and committed it on all branches.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development