On 03/09/22 20:46, Martijn Dashorst wrote:
I've added PRs for wicket 8.x, 9.x and master. I assume we won't be
releasing a 7.x anytime soon, so this is just one more benefit for
upgrading to a newer version IMO.
Even though this came up as a blocking hotspot, it did not show up as a
performance hotspot, so it will benefit all wicket applications, but it is
not detrimental.
If there are no objections I intend to merge these changes somewhere on
monday.
Martijn
Ok! Thank you!
On Fri, Sep 2, 2022 at 5:43 PM Martijn Dashorst <martijn.dasho...@gmail.com>
wrote:
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
