RFR: 8339114: DaCapo xalan performance with -XX:+UseObjectMonitorTable [v3]

[Coleen Phillimore]

On Wed, 26 Mar 2025 07:26:19 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Looks like from GHA, this also fails the same test.  I think you should remove this optimization since I don't think it helps any.
>
> 'it does not help any' is not correct. The point of this PR is to put the OM in the BasicLock cache early, even if it does not succeed to enter, so that slower paths can pick it up from there. Also, simply removing this particular line would not change anything, because we put the OM in the BasicLock cache in the C2 fast-path in the same way.
> 
> I think the BasicLock OM cache should be treated the same way as the thread's OMCache. The BasicLock cache is basically the L1 cache, while the OMCache is the L2 cache. And just like the OMCache lookup, we should clear the BasicLock cache when we observe a deflating monitor. This fixes the failing test.
> @xmas92 should also look at this.

Yes, the concept of a L1 and L2 cache is good and clear.  And does help performance of look ups in the OMCache.  I reconstructed the crash I got with the runtime/Monitor/UseObjectMonitorTableTest.java#ExtremeDeflation test.  I was setting the OMCache also with the monitor found, because I was only trying to avoid redoing the lookup in the table itself, but in this test, we find a deflated monitor in the OMCache, and then got the assert that it's not in the table.

I don't know if you want to recheck if the monitor is deflated at this time, even though it might be racy.  It's unlikely and the check and clearing in BasicLock is better.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24098#discussion_r2017589913