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
