Thanks for the review, Michael! SerializedPagesCache is the cache that is used in 6.14.0 with the problems explained in the first mail in this thread. Its initial max size is 40 (org.apache.wicket.settings.def.StoreSettings#DEFAULT_CACHE_SIZE). That it it holds the last 40 used pages in the whole application (i.e. in all http sessions).
The improvement is that it doesn't use an array (thru ArrayList) as a backing store, so it is not fully reconstructed on every #remove operation, and that now it is a concurrent data structure (ConcurrentLinkedDeque, Java 7 !) instead of a synchronized field. The new impl - PerSessionPageStore again uses concurrent data structures for its cache impl but it holds the last N pages per session, so even if you set N to 1 your will have 10K cached pages if your have 10K active http sessions. The other difference is that PerSessionPageStore works with live page instances while DefaultPageStore works with SerializedPage class, i.e. with the byte[] view of a page. It does this to be prepared for http session replication initiated by the web container, so there is no need of a second serialization of the same page. (The first serialization is to store the page in DiskDataStore). Martin Grigorov Wicket Training and Consulting On Wed, Mar 12, 2014 at 10:56 AM, Michael Mosmann <[email protected]>wrote: > Am 11.03.14 13:55, schrieb Martin Grigorov: > >> Any comments on >> https://github.com/apache/wicket/compare/5527-inefficient- >> DefaultDataStoreare >> very welcome! >> > Hmm.. SerializedPagesCache .. if one has many pages costs will go up.. but > i am not sure, how big the cache will be in real life applications.. > > It should be relatively easy to roll custom impls based on >> Guava/Hazelcast/... if needed/preferred. >> > yes.. it hast enought extension points in your version.. > > .. > ok for me.. looks good:) > > mm:) > > Martin Grigorov >> Wicket Training and Consulting >> >> >> On Mon, Mar 10, 2014 at 9:55 AM, Martin Grigorov <[email protected] >> >wrote: >> >> I can see the benefits of adding dependency to Guava, but I can also see >>> many dependency conflicts caused by this. >>> Guava is not that strict in backward compatibility and this will lead to >>> problems like: an application depends on version X because of feature X1 >>> and Wicket requires version Y that is incompatible. >>> >>> >>> On Fri, Mar 7, 2014 at 7:18 PM, Andrea Del Bene <[email protected] >>> >wrote: >>> >>> Having a flexible eviction policy would be nice, but if we decide to for >>>> this way I would strongly recommend to NOT start implementing our own >>>> cache solution. Instead, we should consider to adopt one of the existing >>>> solutions (like Guava cache). >>>> >>>>> An additional consideration: what about getPage(String sessionId, int >>>>> >>>> pageId), should this also update the modification time (or maybe better: >>>> „lastAccessTime“) of the returned page? Pages that are more frequently >>>> requested maybe should be removed later from the cache …!? >>>> >>>>> >>>>> Your suggestion would be much simpler to implement but the total size >>>>>> >>>>> of >>>> >>>>> this cache will be dynamic and will depend on the number of the active >>>>>> sessions. But since this cache uses SoftReference for the values (the >>>>>> pages) I think this won't be a problem. >>>>>> >>>>> We could have a (configurable) global limit for all cached pages in all >>>>> >>>> sessions, if that limit is reached, the least recently accessed/modified >>>> page could be removed … >>>> >>>>> >>>>> Cheers, >>>>> -Tom >>>>> >>>>> >>>>> >>>> >
