I've created this ticket for this: https://issues.apache.org/jira/browse/WICKET-7002
And a PR: https://github.com/apache/wicket/pull/532 Martijn On Fri, Sep 2, 2022 at 2:37 PM Martijn Dashorst <martijn.dasho...@gmail.com> wrote: > Well, > > I think that the metadata storage in Application tries to mimic a > key-value store, but does so in an esoteric way that requires > synchronization, which is unnecessary when we just use a ConcurrentHashMap > for the datastructure that is especially designed for this use case. > Changing the array to a ConcurrentHashMap will not increase the memory > footprint in any meaningful way, but will remove the need for > synchronization and change the algorithm for looking up a value by key from > O(n) to O(1) ( > https://stackoverflow.com/questions/559839/big-o-summary-for-java-collections-framework-implementations > ). > > The change is completely encapsulated for external users (unless they play > with the internals using reflection) so this can also be backported to > Wicket 9, 8 and 7, up til Wicket 1.5 if we so desire (we don't but you get > my drift). > > I also think it is a bad idea to return the datastructure from the setter. > This would have made this optimization impossible to do in a backwards > compatible way. > > Martijn > > > On Fri, Sep 2, 2022 at 11:59 AM Andrea Del Bene <an.delb...@gmail.com> > wrote: > >> It's of course a delicate topic and it needs a full investigation, but >> looking at the code of both setMetaData and getMetaData I think we could >> reduce the synchronization overloading by making Application#metadata >> volatile and removing synchronized from getMetaData. In addition I think >> setMetaData should always return a new copy of Application#metadata to >> ensure a proper synchronization among threads, but I'm not 100% sure about >> this. >> >> Just some 2 cents... >> >> On Fri, Sep 2, 2022 at 8:46 AM Martijn Dashorst < >> martijn.dasho...@gmail.com> >> wrote: >> >> > Heya! >> > >> > In a performance dump for a production issue I was looking through the >> > monitors that were in use during request processing and I noticed that a >> > lot of request threads were blocking on Application.getMetaData. >> > >> > This method is synchronized because it uses an array of metadata and >> goes >> > through that array in search of the key. >> > >> > I'm no rocket scientist, but shouldn't that metadata field just be a >> > ConcurrentHashMap so we can remove the blocking synchronization on the >> > application object? >> > >> > Martijn >> > >> > <http://wicketinaction.com> >> > >> >> >> -- >> Andrea Del Bene. >> Apache Wicket committer. >> > > > -- > Become a Wicket expert, learn from the best: http://wicketinaction.com > -- Become a Wicket expert, learn from the best: http://wicketinaction.com
