You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2014/03/06 15:01:00 UTC
Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
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
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
We could have second impl of org.apache.wicket.pageStore.IPageStore and you
can switch them by
overriding org.apache.wicket.DefaultPageManagerProvider#newPageStore
The question is: how useful this second level cache really is ?
The first level cache (all touched pages in the last request cycle being
kept in the http session) is useful because every Ajax interaction re-uses
the live page instance from this cache.
The second and third level caches are used when the user goes back in the
history.
Until now the second level cache (the one we discuss) keeps the last N
*serialized *pages for all users.
If the second level cache doesn't have entry for sessionId+pageId key then
the third level (the disk) is used.
In our application we have very low read ratio for the second and third
level caches. Our users mostly navigate always forward, i.e. they use the
links in the application.
On Fri, Mar 7, 2014 at 4:47 PM, Nick Pratt <nb...@gmail.com> wrote:
> Would it be possible to make this change available as a switchable option
> in 6.x (in addition to 7.x), leaving the current store/mechanism in place.
>
> The reason I ask is because this seems to be a very significant fundamental
> change to Wicket, and while the analysis by the original poster was very
> thorough and detailed, and the changes discussed so far sound very
> promising, I'd like to try it out on existing apps, without being forced to
> upgrade to 7.x due to the core nature of the changes being proposed. So if
> we could switch the implementations (existing vs proposed) via a setting in
> Application::init this would be extremely helpful for testing memory usage
> and performance impacts.
>
> N
>
>
>
> On Fri, Mar 7, 2014 at 9:14 AM, Tom Götz <to...@decoded.de> wrote:
>
> >
> > On 07.03.2014, at 14:14, Martin Grigorov <mg...@apache.org> wrote:
> >
> > > On Fri, Mar 7, 2014 at 2:51 PM, Andrea Del Bene <an.delbene@gmail.com
> > >wrote:
> > >
> > >> 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.
> > >>
> > >
> > > This changes the behavior completely but I tend to agree with you.
> > > At the moment I use ConcurrentSkipListMap<SessionId,
> > > ConcurrentSkipListMap<PageId, IManageablePage>> , where SessionId class
> > > wraps the String sessionId and has modificationTime that is used to
> sort
> > > the entries so I can remove the oldest one when the limit is reached.
> > Same
> > > for PageId.
> > > The problem is that with this approach I will always remove the oldest
> > Page
> > > from the oldest Session. But there may be even older Page in another
> > > Session that is added later. So it is not correct...
> >
> > 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
> >
> >
> >
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Nick Pratt <nb...@gmail.com>.
Would it be possible to make this change available as a switchable option
in 6.x (in addition to 7.x), leaving the current store/mechanism in place.
The reason I ask is because this seems to be a very significant fundamental
change to Wicket, and while the analysis by the original poster was very
thorough and detailed, and the changes discussed so far sound very
promising, I'd like to try it out on existing apps, without being forced to
upgrade to 7.x due to the core nature of the changes being proposed. So if
we could switch the implementations (existing vs proposed) via a setting in
Application::init this would be extremely helpful for testing memory usage
and performance impacts.
N
On Fri, Mar 7, 2014 at 9:14 AM, Tom Götz <to...@decoded.de> wrote:
>
> On 07.03.2014, at 14:14, Martin Grigorov <mg...@apache.org> wrote:
>
> > On Fri, Mar 7, 2014 at 2:51 PM, Andrea Del Bene <an.delbene@gmail.com
> >wrote:
> >
> >> 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.
> >>
> >
> > This changes the behavior completely but I tend to agree with you.
> > At the moment I use ConcurrentSkipListMap<SessionId,
> > ConcurrentSkipListMap<PageId, IManageablePage>> , where SessionId class
> > wraps the String sessionId and has modificationTime that is used to sort
> > the entries so I can remove the oldest one when the limit is reached.
> Same
> > for PageId.
> > The problem is that with this approach I will always remove the oldest
> Page
> > from the oldest Session. But there may be even older Page in another
> > Session that is added later. So it is not correct...
>
> 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
>
>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Michael Mosmann <mi...@mosmann.de>.
Hi,
.. is the link correct?
mm:)
Am 11.03.14 13:55, schrieb Martin Grigorov:
> Any comments on
> https://github.com/apache/wicket/compare/5527-inefficient-DefaultDataStoreare
> very welcome!
>
> It should be relatively easy to roll custom impls based on
> Guava/Hazelcast/... if needed/preferred.
>
> Martin Grigorov
> Wicket Training and Consulting
>
>
> On Mon, Mar 10, 2014 at 9:55 AM, Martin Grigorov <mg...@apache.org>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 <an...@gmail.com>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
>>>>
>>>>
>>>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Michael Mosmann <mi...@mosmann.de>.
ah.. ok. then this should work fine in most cases..
mm:)
Am 12.03.14 10:13, schrieb Martin Grigorov:
> 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 <mi...@mosmann.de>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 <mgrigorov@apache.org
>>>> 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 <an.delbene@gmail.com
>>>>> 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
>>>>>>
>>>>>>
>>>>>>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
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 <mi...@mosmann.de>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 <mgrigorov@apache.org
>> >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 <an.delbene@gmail.com
>>> >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
>>>>>
>>>>>
>>>>>
>>>>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Michael Mosmann <mi...@mosmann.de>.
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 <mg...@apache.org>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 <an...@gmail.com>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
>>>>
>>>>
>>>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
On Wed, Mar 12, 2014 at 10:40 AM, Michael Mosmann <mi...@mosmann.de>wrote:
> a.. got it..
>
> https://github.com/apache/wicket/compare/master...5527-
> inefficient-DefaultDataStore
>
> Am 11.03.14 13:55, schrieb Martin Grigorov:
>
>> Any comments on
>> https://github.com/apache/wicket/compare/5527-inefficient-
>> DefaultDataStoreare
>
>
Not sure why in your version of the link "are" is appended to the link and
breaks it.
If you remove it manually then it is also valid.
>
>> very welcome!
>>
>> It should be relatively easy to roll custom impls based on
>> Guava/Hazelcast/... if needed/preferred.
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>>
>>
>> On Mon, Mar 10, 2014 at 9:55 AM, Martin Grigorov <mgrigorov@apache.org
>> >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 <an.delbene@gmail.com
>>> >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
>>>>>
>>>>>
>>>>>
>>>>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Michael Mosmann <mi...@mosmann.de>.
a.. got it..
https://github.com/apache/wicket/compare/master...5527-inefficient-DefaultDataStore
Am 11.03.14 13:55, schrieb Martin Grigorov:
> Any comments on
> https://github.com/apache/wicket/compare/5527-inefficient-DefaultDataStoreare
> very welcome!
>
> It should be relatively easy to roll custom impls based on
> Guava/Hazelcast/... if needed/preferred.
>
> Martin Grigorov
> Wicket Training and Consulting
>
>
> On Mon, Mar 10, 2014 at 9:55 AM, Martin Grigorov <mg...@apache.org>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 <an...@gmail.com>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
>>>>
>>>>
>>>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
Any comments on
https://github.com/apache/wicket/compare/5527-inefficient-DefaultDataStoreare
very welcome!
It should be relatively easy to roll custom impls based on
Guava/Hazelcast/... if needed/preferred.
Martin Grigorov
Wicket Training and Consulting
On Mon, Mar 10, 2014 at 9:55 AM, Martin Grigorov <mg...@apache.org>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 <an...@gmail.com>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
>> >
>> >
>>
>>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
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 <an...@gmail.com>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
> >
> >
>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Andrea Del Bene <an...@gmail.com>.
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
>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Tom Götz <to...@decoded.de>.
On 07.03.2014, at 14:14, Martin Grigorov <mg...@apache.org> wrote:
> On Fri, Mar 7, 2014 at 2:51 PM, Andrea Del Bene <an...@gmail.com>wrote:
>
>> 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.
>>
>
> This changes the behavior completely but I tend to agree with you.
> At the moment I use ConcurrentSkipListMap<SessionId,
> ConcurrentSkipListMap<PageId, IManageablePage>> , where SessionId class
> wraps the String sessionId and has modificationTime that is used to sort
> the entries so I can remove the oldest one when the limit is reached. Same
> for PageId.
> The problem is that with this approach I will always remove the oldest Page
> from the oldest Session. But there may be even older Page in another
> Session that is added later. So it is not correct…
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
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
On Fri, Mar 7, 2014 at 2:51 PM, Andrea Del Bene <an...@gmail.com>wrote:
> 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.
>
This changes the behavior completely but I tend to agree with you.
At the moment I use ConcurrentSkipListMap<SessionId,
ConcurrentSkipListMap<PageId, IManageablePage>> , where SessionId class
wraps the String sessionId and has modificationTime that is used to sort
the entries so I can remove the oldest one when the limit is reached. Same
for PageId.
The problem is that with this approach I will always remove the oldest Page
from the oldest Session. But there may be even older Page in another
Session that is added later. So it is not correct...
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.
What other devs think ?
>
> 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
> >
>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Andrea Del Bene <an...@gmail.com>.
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
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Guillaume Smet <gu...@gmail.com>.
On Wed, Apr 9, 2014 at 9:27 AM, Martin Grigorov <mg...@apache.org> wrote:
> The second level cache is disabled now -
> https://issues.apache.org/jira/browse/WICKET-5554
Thanks Martin!
--
Guillaume
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
The second level cache is disabled now -
https://issues.apache.org/jira/browse/WICKET-5554
Martin Grigorov
Wicket Training and Consulting
On Tue, Apr 8, 2014 at 10:04 AM, Martin Grigorov <mg...@apache.org>wrote:
> More opinions here ?
>
> Martin Grigorov
> Wicket Training and Consulting
>
>
> On Mon, Apr 7, 2014 at 1:01 PM, Sven Meier <sv...@meiers.net> wrote:
>
>> +1
>>
>> I think this is the right direction. In the long term we should revisit
>> some decisions/relicts of storing pages in Wicket.
>>
>> Sven
>>
>>
>> On 04/04/2014 03:19 PM, Martin Grigorov wrote:
>>
>>> Hi Guillaume,
>>>
>>> We have also disabled the second level cache for our main application for
>>> the time being.
>>>
>>> Maybe we should set 0 as the default cache size for 6.15.0 and explain
>>> this
>>> in the announcement + a blog + some tweets ?
>>> If an application wants to use the second level cache then it should
>>> enable
>>> it explicitly.
>>>
>>> What other Wicket devs/users think ?
>>>
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>>
>>>
>>> On Fri, Apr 4, 2014 at 3:12 PM, Guillaume Smet <guillaume.smet@gmail.com
>>> >wrote:
>>>
>>> Hi Martin,
>>>>
>>>> Some feedback you might find useful about this.
>>>>
>>>> On Thu, Mar 6, 2014 at 3:01 PM, Martin Grigorov <mg...@apache.org>
>>>> wrote:
>>>>
>>>>> A workaround to avoid the slowness caused by this is to set 0 or
>>>>> negative
>>>>> value to org.apache.wicket.settings.StoreSettings#setInmemoryCacheSize
>>>>>
>>>> We have a quite big application which was slow under load without us
>>>> being able to find the culprit.
>>>>
>>>> I set the InmemoryCacheSize to 0 yesterday and the application is now
>>>> much more reactive.
>>>>
>>>> We use a lot the disk data store as we have back links nearly
>>>> everywhere and clicking back isn't slower than before. Probably
>>>> because we couldn't set the cache too high due to memory issues and we
>>>> probably have too many users to have an effective inmemorycache with
>>>> the size we configured.
>>>>
>>>> Might be useful to spread the word about it.
>>>>
>>>> Thanks for your post on this subject.
>>>>
>>>> --
>>>> Guillaume
>>>>
>>>>
>>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
More opinions here ?
Martin Grigorov
Wicket Training and Consulting
On Mon, Apr 7, 2014 at 1:01 PM, Sven Meier <sv...@meiers.net> wrote:
> +1
>
> I think this is the right direction. In the long term we should revisit
> some decisions/relicts of storing pages in Wicket.
>
> Sven
>
>
> On 04/04/2014 03:19 PM, Martin Grigorov wrote:
>
>> Hi Guillaume,
>>
>> We have also disabled the second level cache for our main application for
>> the time being.
>>
>> Maybe we should set 0 as the default cache size for 6.15.0 and explain
>> this
>> in the announcement + a blog + some tweets ?
>> If an application wants to use the second level cache then it should
>> enable
>> it explicitly.
>>
>> What other Wicket devs/users think ?
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>>
>>
>> On Fri, Apr 4, 2014 at 3:12 PM, Guillaume Smet <gu...@gmail.com>
>> wrote:
>>
>> Hi Martin,
>>>
>>> Some feedback you might find useful about this.
>>>
>>> On Thu, Mar 6, 2014 at 3:01 PM, Martin Grigorov <mg...@apache.org>
>>> wrote:
>>>
>>>> A workaround to avoid the slowness caused by this is to set 0 or
>>>> negative
>>>> value to org.apache.wicket.settings.StoreSettings#setInmemoryCacheSize
>>>>
>>> We have a quite big application which was slow under load without us
>>> being able to find the culprit.
>>>
>>> I set the InmemoryCacheSize to 0 yesterday and the application is now
>>> much more reactive.
>>>
>>> We use a lot the disk data store as we have back links nearly
>>> everywhere and clicking back isn't slower than before. Probably
>>> because we couldn't set the cache too high due to memory issues and we
>>> probably have too many users to have an effective inmemorycache with
>>> the size we configured.
>>>
>>> Might be useful to spread the word about it.
>>>
>>> Thanks for your post on this subject.
>>>
>>> --
>>> Guillaume
>>>
>>>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Sven Meier <sv...@meiers.net>.
+1
I think this is the right direction. In the long term we should revisit
some decisions/relicts of storing pages in Wicket.
Sven
On 04/04/2014 03:19 PM, Martin Grigorov wrote:
> Hi Guillaume,
>
> We have also disabled the second level cache for our main application for
> the time being.
>
> Maybe we should set 0 as the default cache size for 6.15.0 and explain this
> in the announcement + a blog + some tweets ?
> If an application wants to use the second level cache then it should enable
> it explicitly.
>
> What other Wicket devs/users think ?
>
> Martin Grigorov
> Wicket Training and Consulting
>
>
> On Fri, Apr 4, 2014 at 3:12 PM, Guillaume Smet <gu...@gmail.com>wrote:
>
>> Hi Martin,
>>
>> Some feedback you might find useful about this.
>>
>> On Thu, Mar 6, 2014 at 3:01 PM, Martin Grigorov <mg...@apache.org>
>> wrote:
>>> A workaround to avoid the slowness caused by this is to set 0 or negative
>>> value to org.apache.wicket.settings.StoreSettings#setInmemoryCacheSize
>> We have a quite big application which was slow under load without us
>> being able to find the culprit.
>>
>> I set the InmemoryCacheSize to 0 yesterday and the application is now
>> much more reactive.
>>
>> We use a lot the disk data store as we have back links nearly
>> everywhere and clicking back isn't slower than before. Probably
>> because we couldn't set the cache too high due to memory issues and we
>> probably have too many users to have an effective inmemorycache with
>> the size we configured.
>>
>> Might be useful to spread the word about it.
>>
>> Thanks for your post on this subject.
>>
>> --
>> Guillaume
>>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Guillaume Smet <gu...@gmail.com>.
On Fri, Apr 4, 2014 at 3:19 PM, Martin Grigorov <mg...@apache.org> wrote:
> Maybe we should set 0 as the default cache size for 6.15.0 and explain this
> in the announcement + a blog + some tweets ?
> If an application wants to use the second level cache then it should enable
> it explicitly.
+1.
It might be useful for an application with very few users but I don't
think it makes much difference in this case to use the disk datastore
and it's definitely harmful when you have a lot of users on your app.
--
Guillaume
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
Hi Guillaume,
We have also disabled the second level cache for our main application for
the time being.
Maybe we should set 0 as the default cache size for 6.15.0 and explain this
in the announcement + a blog + some tweets ?
If an application wants to use the second level cache then it should enable
it explicitly.
What other Wicket devs/users think ?
Martin Grigorov
Wicket Training and Consulting
On Fri, Apr 4, 2014 at 3:12 PM, Guillaume Smet <gu...@gmail.com>wrote:
> Hi Martin,
>
> Some feedback you might find useful about this.
>
> On Thu, Mar 6, 2014 at 3:01 PM, Martin Grigorov <mg...@apache.org>
> wrote:
> > A workaround to avoid the slowness caused by this is to set 0 or negative
> > value to org.apache.wicket.settings.StoreSettings#setInmemoryCacheSize
>
> We have a quite big application which was slow under load without us
> being able to find the culprit.
>
> I set the InmemoryCacheSize to 0 yesterday and the application is now
> much more reactive.
>
> We use a lot the disk data store as we have back links nearly
> everywhere and clicking back isn't slower than before. Probably
> because we couldn't set the cache too high due to memory issues and we
> probably have too many users to have an effective inmemorycache with
> the size we configured.
>
> Might be useful to spread the word about it.
>
> Thanks for your post on this subject.
>
> --
> Guillaume
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Guillaume Smet <gu...@gmail.com>.
Hi Martin,
Some feedback you might find useful about this.
On Thu, Mar 6, 2014 at 3:01 PM, Martin Grigorov <mg...@apache.org> wrote:
> A workaround to avoid the slowness caused by this is to set 0 or negative
> value to org.apache.wicket.settings.StoreSettings#setInmemoryCacheSize
We have a quite big application which was slow under load without us
being able to find the culprit.
I set the InmemoryCacheSize to 0 yesterday and the application is now
much more reactive.
We use a lot the disk data store as we have back links nearly
everywhere and clicking back isn't slower than before. Probably
because we couldn't set the cache too high due to memory issues and we
probably have too many users to have an effective inmemorycache with
the size we configured.
Might be useful to spread the word about it.
Thanks for your post on this subject.
--
Guillaume
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Martin Grigorov <mg...@apache.org>.
On Fri, Mar 7, 2014 at 1:21 PM, Tom Götz <to...@decoded.de> wrote:
> Just curious:
> in 5527-inefficient-DefaultDataStore branch (8908b8f2) you are now using a
> ConcurrentLinkedDeque as a cache. Why not use a map here, e.g.
> ConcurrentSkipListMap with entries ordered by a timestamp or whatever?
>
This is what I'm doing at the moment actually! :-)
Just replacing ArrayList with ConcurrentLinkedDeque was the low hanging one.
With CSLMap it is a bit more tricky to find the oldest entry in
Map<String-SessionId, Map<Int-PageId, Page>> but not impossible.
>
> Plus: why is that cache global in terms of that it contains all pages of
> all sessions? Couldn’t we have a cache that stores pages per sessionId?
>
This is a very good question!
I was not involved in the design and implementation of these classes, so I
don't know why exactly it works like this. I just know that it works this
way consistently - StoredResponseMap (used by REDIRECT_TO_BUFFER) strategy
also stores on application level.
My guess is that the Session as part of the http session is replicated and
storing this cache there will lead to more bytes being serialized when the
web container does its replication logic.
>
> Asking because I’m currently implementing an own IPageStore and want to
> learn about the mechanics and possible pitfalls.
>
Thanks for looking at this code and asking !
>
> Cheers,
> -Tom
>
>
> On 06.03.2014, at 15:01, Martin Grigorov <mg...@apache.org> wrote:
>
> > 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.
>
>
Re: Inefficient org.apache.wicket.pageStore.DefaultPageStore.SerializedPagesCache
Posted by Tom Götz <to...@decoded.de>.
Just curious:
in 5527-inefficient-DefaultDataStore branch (8908b8f2) you are now using a ConcurrentLinkedDeque as a cache. Why not use a map here, e.g. ConcurrentSkipListMap with entries ordered by a timestamp or whatever?
Plus: why is that cache global in terms of that it contains all pages of all sessions? Couldn’t we have a cache that stores pages per sessionId?
Asking because I’m currently implementing an own IPageStore and want to learn about the mechanics and possible pitfalls.
Cheers,
-Tom
On 06.03.2014, at 15:01, Martin Grigorov <mg...@apache.org> wrote:
> 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.