[jira] Closed: (WICKET-2889) Remove reference to last page from idle SecondLevelCachePageMap

["Igor Vaynberg (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/WICKET-2889?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg closed WICKET-2889.
---------------------------------

      Assignee: Johan Compagner
    Resolution: Won't Fix

From: Johan Compagner <jc...@gmail.com>
Date: Mon, 9 Aug 2010 17:00:12 +0200
Message-ID: <AA...@mail.gmail.com>
Subject: Re: Could somebody please comment on WICKET-2889?
To: users@wicket.apache.org
Content-Type: text/plain; charset=ISO-8859-1

I am against that patch.
You keep pagemaps in memory in the applicaiton context!
Those are session stuff stored in the HttpSession. You shouldnt keep
reference to those stuff.
This can break all kind of things (for example clustering)

If you want something like that, then it is fine if we need to change
something so that you can overwrite some stuff so that you can do what
you want without patching wicket.
But i dont want this to be default in wicket.



On Mon, Aug 9, 2010 at 16:43, Stefan Fussenegger <st...@molindo.at> wrote:
> Hi all,
>
> It's been more than 2 month since I've created WICKET-2889 and submitted a
> patch to fix it. As nobody commented yet, I thought I should mention it
> here.
>
> The attached patch reduced required heap space for stateful pages by 2/3
> (depends on configuration though, >100M for my application running with a
> total heap of 768M) without any noticeable impact on performance. I'm using
> a custom PageMap that requires a few trivial changes in
> SecondLevelCachePageMap (visibility and a hook called
> onAfterLastPageSet(..)). I doubt that there's any reason not to apply it for
> 1.4.10. Whether to include the new class
> (ExpiringSecondLevelCacheSessionStore) is optional, but I'd consider it a
> valuable addition to core.
>
> Please see https://issues.apache.org/jira/browse/WICKET-2889
> <https://issues.apache.org/jira/browse/WICKET-2361>
> <https://issues.apache.org/jira/browse/WICKET-2361> for details.
>
> Cheers, Stefan
>

> Remove reference to last page from idle SecondLevelCachePageMap 
> ----------------------------------------------------------------
>
>                 Key: WICKET-2889
>                 URL: https://issues.apache.org/jira/browse/WICKET-2889
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket
>    Affects Versions: 1.4.8
>         Environment: Wicket 1.4.8, Java 1.6.0_20 amd64, Ubuntu Hardy
>            Reporter: Stefan Fussenegger
>            Assignee: Johan Compagner
>            Priority: Minor
>         Attachments: wicket-2889.patch
>
>
> Currently, the last used page per PageMap is kept in memory to avoid deserialization overhead. After looking at a heap dump of my application, I found that this strategy is pretty memory intensive: about 170M for 1500 session (115K per session) in the case of my application.
> To reduce this overhead, I'd suggest to drop the last page for "idle" PageMaps. A PageMap could be considered idle as soon as a user doesn't access a page for 10 minutes. I'd argue that it's pretty unlikely that a user accesses any page after 10 minutes (admittedly depending on the application itself) anyway. In cases the user becomes active again, the deserialization overhead should be acceptable. As our application's sessions expire after 30 minutes, this feature would save 2/3 or about 113M of 170M on memory.
> public void expireLastPage() {
>   if (sessionId != null && lastPage instanceof Page) {
>     IPageStore store = getPageStore();
>     if (store instanceof ISerializationAwarePageStore) {
>       lastPage = ((ISerializationAwarePageStore)store).prepareForSerialization(sessionId, page)
>     }
>   }
> }
> A LinkedHashMap at application scope could be used to expire idle PageMaps, e.g. using
> private void setLastPage(Page lastPage) {
>   this.lastPage = lastPage;
>   // (re)add to map
>   LinkedHashMap<SecondLevelCachePageMap, Long> map = Application.get().getMetaData(LAST_PAGE_EXPIRE_MAP); 
>   map.remove(this);
>   map.put(this, System.currentTimeMillis());
> }
> A thread could then periodically check for idle PageMaps:
> void run() {
>   Iterator<Map.Entry<SecondLevelCachePageMap, Long>> iter = Application.get().getMetaData(LAST_PAGE_EXPIRE_MAP).iterator();
>   while (iter.hasNext()) {
>     Map.Entry<SecondLevelCachePageMap, Long> e = iter.next();
>     if (isIdle(e.getValue())) break;
>     e.getKey().expireLastPage();
>     iter.remove();
>   }
> }
> (the code is just to clarify the idea. for synchronization and encapsulation purposes, the map should certainly be wrapped inside a new class).
> Alternatively, pages could be expired if a configured capacity of the map is exceeded. In this case it would be pretty hard to select a sane default that suits most applications out of the box.
> Please comment if this sound like an acceptable addition. I'd be happy to submit a patch (or at least a a patch that would allow to easily extend the current implementation). I'd really appreciate it this feature would find it's way into 1.4.x, preferably as optionally configurable.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.