javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-1557

ResourceHandlerImpl does not cache in production mode

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.0.3-b01
    • Component/s: resources
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      1,557

      Description

      ResourceHandlerImpl.handleResourceRequest takes too long to deliver a 304. From
      my point of view, delivering a 304 should take quasi no time.

      The problem seems to be that ResourceManager.findLibrary is called again and
      again. Can resources really change in production mode?

      If resources can't change in production mode, simply cache all necessary
      information (most notably last modified) and return 304 immediately.

      I will look deeper into this. Maybe i will find some point to change.

        Activity

        Frank Caputo created issue -
        Hide
        Ryan Lubke added a comment -

        Resources can change in production mode. If you don't plan on having them
        change, then set the context init parameter
        'com.sun.faces.resourceUpdateCheckPeriod' to -1.

        Regardless, the library will always have to be looked up (assuming the library
        name was included with the resource url) with the potential of a cached result
        returned.

        Show
        Ryan Lubke added a comment - Resources can change in production mode. If you don't plan on having them change, then set the context init parameter 'com.sun.faces.resourceUpdateCheckPeriod' to -1. Regardless, the library will always have to be looked up (assuming the library name was included with the resource url) with the potential of a cached result returned.
        Hide
        Ed Burns added a comment -

        I have observed this as well. It's a problem.

        Show
        Ed Burns added a comment - I have observed this as well. It's a problem.
        Hide
        Frank Caputo added a comment -

        Will you fix it?

        Show
        Frank Caputo added a comment - Will you fix it?
        Hide
        Ed Burns added a comment -

        Before we go further, if you set 'com.sun.faces.resourceUpdateCheckPeriod' to -1,
        is the performance acceptable?

        Show
        Ed Burns added a comment - Before we go further, if you set 'com.sun.faces.resourceUpdateCheckPeriod' to -1, is the performance acceptable?
        Hide
        Ryan Lubke added a comment -

        Assigned.

        Show
        Ryan Lubke added a comment - Assigned.
        Hide
        Ryan Lubke added a comment -

        Guess I'm not seeing an issue here.

        The 304 response is independent of ResourceManager interactions. Based on the
        API, the Resource has to be looked up before we can determine if the user agent
        needs update.

        This means we have to go through the ResourceManager to obtain the cached
        metadata. Note that by default, the metadata will be refreshed every 5 minutes
        from its last access unless the configuration option I referenced previously is
        set to -1.

        However, anytime the user agent asks about the modified time of a resource,
        we'll answer it.

        Doing some local testing with FF 3.6 and Live HTTP Headers, if I make an initial
        request to an application with managed resources, I see the GET being made to
        the resources with the Expires, Last-Modified, and ETag response headers being
        sent.

        If I then issue the same GET request, I see only the GET for the page, and not
        the resources, so the conditional GET sequence is not initiated. Assuming no
        reloads, the request for the resource shouldn't be issues again for another
        seven days.

        If the reload button is hit, then a request is sent for the resources with an
        If-Modified-Since request header. This cases
        ResourceImpl.userAgentNeedsUpdate() to be invoked which will query the timestamp.

        If a shift-reload occurs, then the resource is re-requested with no
        If-Modified-Since header.

        If you're seeing the conditional GET requests being sent for every resource
        regardless of the headers, I'd like to know what user agent you're using.

        Show
        Ryan Lubke added a comment - Guess I'm not seeing an issue here. The 304 response is independent of ResourceManager interactions. Based on the API, the Resource has to be looked up before we can determine if the user agent needs update. This means we have to go through the ResourceManager to obtain the cached metadata. Note that by default, the metadata will be refreshed every 5 minutes from its last access unless the configuration option I referenced previously is set to -1. However, anytime the user agent asks about the modified time of a resource, we'll answer it. Doing some local testing with FF 3.6 and Live HTTP Headers, if I make an initial request to an application with managed resources, I see the GET being made to the resources with the Expires, Last-Modified, and ETag response headers being sent. If I then issue the same GET request, I see only the GET for the page, and not the resources, so the conditional GET sequence is not initiated. Assuming no reloads, the request for the resource shouldn't be issues again for another seven days. If the reload button is hit, then a request is sent for the resources with an If-Modified-Since request header. This cases ResourceImpl.userAgentNeedsUpdate() to be invoked which will query the timestamp. If a shift-reload occurs, then the resource is re-requested with no If-Modified-Since header. If you're seeing the conditional GET requests being sent for every resource regardless of the headers, I'd like to know what user agent you're using.
        Hide
        Frank Caputo added a comment -

        I also have no problem, if firefox doesn't ask for the resources. But if he
        asks, delivering a 304 takes too long, even if i set the check period to -1.

        The problem is obviously that ResourceHelper.getLastModified always opens a
        connection to get the timestamp and makes unnecessary IO even if i disable
        resource refreshing.

        If resource refreshing is disabled, you could simply remember the last modified
        date in the ResourceInfo and return it immediately without doing IO.

        I think this would help a lot.

        Show
        Frank Caputo added a comment - I also have no problem, if firefox doesn't ask for the resources. But if he asks, delivering a 304 takes too long, even if i set the check period to -1. The problem is obviously that ResourceHelper.getLastModified always opens a connection to get the timestamp and makes unnecessary IO even if i disable resource refreshing. If resource refreshing is disabled, you could simply remember the last modified date in the ResourceInfo and return it immediately without doing IO. I think this would help a lot.
        Hide
        Ryan Lubke added a comment -

        Created an attachment (id=1133)
        Proposed changes (ver 1)

        Show
        Ryan Lubke added a comment - Created an attachment (id=1133) Proposed changes (ver 1)
        Hide
        Jason Lee added a comment -

        r=jdlee

        Show
        Jason Lee added a comment - r=jdlee
        Hide
        Ryan Lubke added a comment -

        Changes applied (r8352).

        We'll cache the timestamp for the lifetime if the ResourceInfo instance,
        however, the cache behavior is disabled by default. The cache behavior can
        be enabled by setting the com.sun.faces.cacheResourceModificationTimestamp
        context init parameter to true.

        Show
        Ryan Lubke added a comment - Changes applied (r8352). We'll cache the timestamp for the lifetime if the ResourceInfo instance, however, the cache behavior is disabled by default. The cache behavior can be enabled by setting the com.sun.faces.cacheResourceModificationTimestamp context init parameter to true.
        Hide
        Ryan Lubke added a comment -

        Mark fixed in b01.

        Show
        Ryan Lubke added a comment - Mark fixed in b01.
        kenaiadmin made changes -
        Field Original Value New Value
        issue.field.bugzillaimportkey 1557 30823
        Hide
        Manfred Riem added a comment -

        Closing issue out

        Show
        Manfred Riem added a comment - Closing issue out
        Manfred Riem made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Ryan Lubke
            Reporter:
            Frank Caputo
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: