Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.3.1
    • Fix Version/s: 1.3.2
    • Component/s: wicket
    • Labels:
      None

      Description

      Currently markup cache uses a cache without any capacity upper bound :| normally this would be ok, holding markup of an entire application doesnt require THAT much memory. However, suppose we have Index page with markup Index.html. the page is requested with local en_us and we cache contents of index.html in the cache. now Index is requested with locale en_gb and we cache it AGAIN. Now we have two entries in the cache caching the exact same thing. the more locales users request (we pull the locale from the user's browser) the more copies of the same thing we cache.

      Thomas Singer reported his application running out of memory, attached memory snapshot contained a MarkupCache instance of over 48MB retained size (the internal map had allocated 4096 keys). He has an application with about 64 pages, and they only support one locale. When I browsed the snapshot I noticed a lot of different locales in the markup cache keys....

      An ideal solution would be to have two caches. One for [container.class,locale,style,variation]>[resource.name] lookup, and the other for [resource.name]>[resource.contents] lookup.

      A suboptimal solution would be to place an upper bound on the cache. See attached patch which contains a 1000 item lru cache. if looks ok we can roll it in as a solution for 1.3.2 branch - since it doesnt require any interfaces breakages.

      1. markuppatch.txt
        7 kB
        Johan Compagner
      2. WICKET-1370.patch.txt
        28 kB
        Igor Vaynberg

        Issue Links

          Activity

          Hide
          Johan Compagner added a comment -

          We pretty much already had the interface we wanted for this (IFixedLocationResourceStream). I only needed to let MarkupResourceStream also implement it.
          And then MarkupCache uses that interface to checkout what the actual file is

          I dont think we will get IResourceStream implementation that are not File or URL and if we do then we fallback on the cachekey only approache.

          It seems to work fine for me when i test locally, the markup is only created once per fixed location and then access through

          cachekey -> locationstring -> markup

          So if locationString doesn't work we get this:

          cachekey -> cachekey -> markup

          But i guess that is a very small percentage if there is even 1 on this planet...

          should we include this into 1.3.2?

          Show
          Johan Compagner added a comment - We pretty much already had the interface we wanted for this (IFixedLocationResourceStream). I only needed to let MarkupResourceStream also implement it. And then MarkupCache uses that interface to checkout what the actual file is I dont think we will get IResourceStream implementation that are not File or URL and if we do then we fallback on the cachekey only approache. It seems to work fine for me when i test locally, the markup is only created once per fixed location and then access through cachekey -> locationstring -> markup So if locationString doesn't work we get this: cachekey -> cachekey -> markup But i guess that is a very small percentage if there is even 1 on this planet... should we include this into 1.3.2?
          Hide
          Johan Compagner added a comment -

          ok i committed it.
          This should be looked at a bit more to see if for example all the interaction with the cache works now
          Especially the reloading/removing of markup

          Show
          Johan Compagner added a comment - ok i committed it. This should be looked at a bit more to see if for example all the interaction with the cache works now Especially the reloading/removing of markup

            People

            • Assignee:
              Johan Compagner
              Reporter:
              Igor Vaynberg
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development