Just my considerations (hopefully not to obvious) on caching inside DefaultPageStore.SerializedPagesCache.
-Using a LinkedList instead of ArrayList would bring better performances because we would avoid to recompating (using add/remove last/first element) -Doing a deeper refactoring we could use one LinkedList for each session and store all of them in a ConcurrentHashMap which supports non-blocking readings. In addition, if a list for a given session is not yet present, we could use method putIfAbsent to ensure that is added only once. With this solution the limit of N-pages would be per session list and not a global value. Hope this could help. > Hi, > > We have identified some problems in > org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache. > > Some history first: > At https://cwiki.apache.org/confluence/display/WICKET/Page+Storage I have > explained how the page storage management works in Wicket 1.5+ > > In brief: > First level cache/store is the HttpSession - here Wicket saves the live > instances of all touched pages in the last request cycle. > Second level cache/store is DefaultPageStore.SerializedPagesCache - here > Wicket saves the last N > (org.apache.wicket.settings.StoreSettings#getInmemoryCacheSize) used pages > in the whole application (by default 40 pages) > Third level cache/store is DiskDataStore - here Wicket stores all pages and > depending on org.apache.wicket.settings.StoreSettings#getMaxSizePerSession > it will "recycle" the file contents > > The identified problems: > > - org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache uses > ArrayList as a data structure to keep SerializedPage instances. When the > limit N (StoreSettings#getInmemoryCacheSize) is reached the ArrayList uses > #remove() to remove the oldest entry. The #remove(0) operation internally > uses System.arraycopy() to compact the internal array structure. As you > already realize this ArrayList is constantly being recompacted in any > application in production. > > - DefaultPageStore.SerializedPagesCache#cache (the same ArrayList) is used > as synchronization monitor for every operation (read/write/remove). I.e. we > have synchronization on application level !! > > - at the moment DefaultPageStore.SerializedPagesCache stores > org.apache.wicket.pageStore.DefaultPageStore.SerializedPage. This is a > structure of {String sessionId, int pageId, byte[] data}. > Since this data is stored in the application scope it is never replicated, > so there is no need to serialize the live page instance to byte[] at all. > Only the third level cache (IDataStore) should work with byte[] > > > A workaround to avoid the slowness caused by this is to set 0 or negative > value to org.apache.wicket.settings.StoreSettings#setInmemoryCacheSize > > > Please take a look at the related code and give your opinion. It is > possible that we didn't understood correctly the purpose of this cache and > its usage. > In the meantime I will start working on optimization. I expect that it will > require API changes in IPageStore interface so it may be only for Wicket 7. > > > > Martin Grigorov > Wicket Training and Consulting >
