Small question about JDK-8253064 and ObjectMonitor allocation

[Thomas Stüfe]

Thanks a lot for your answers, David.

On Mon, Jan 31, 2022 at 8:35 AM David Holmes <david.holmes at oracle.com>
wrote:

> On 31/01/2022 3:54 pm, Thomas Stüfe wrote:
> > Hi David,
> >
> > Thank you for the answer!
> >
> > On Mon, Jan 31, 2022 at 6:23 AM David Holmes <david.holmes at oracle.com
> > <mailto:david.holmes at oracle.com>> wrote:
> >
> >     Hi Thomas,
> >
> >     On 31/01/2022 2:32 pm, Thomas Stüfe wrote:
> >      > Hi,
> >      >
> >      > I have a small question about a detail of JDK-8253064.
> >      >
> >      > IIUC, before this patch, the VM kept thread-local freelists of
> >      > pre-allocated ObjectMonitors to reduce allocation contention. Now
> >     we just
> >      > malloc monitors right away.
> >      >
> >      > I looked through the issue and the associated PR, but could find
> no
> >      > information on why this was done. Dan describes what he did very
> >     well:
> >      > https://github.com/openjdk/jdk/pull/642#issuecomment-720753946
> >     <https://github.com/openjdk/jdk/pull/642#issuecomment-720753946>,
> >     but not
> >      > why.
> >      >
> >      > I assume that the complexity and memory overhead of the free
> >     lists was not
> >      > worth it? That you found that malloc() is on our platforms
> >     "uncontented"
> >      > enough?
> >
> >     The issue was not about freelists and contention it was about
> requiring
> >     type-stable-memory: that once a piece of memory was allocated as an
> >     ObjectMonitor it remained forever after an ObjectMonitor. This
> allowed
> >     for various race conditions in the old monitor code maintaining
> safety.
> >     Over time that code changed substantially and the need for
> >     type-stable-memory for ObjectMonitors disappeared, so we finally got
> >     rid
> >     of it and just moved to a direct allocation scheme.
> >
> >
> > I think I understand, but I was specifically concerned with the question
> > of allocation contention of ObjectMonitors. That is somewhat independent
> > from the question of where OMs are allocated.
> >
> > Can it happen that lock inflation happens clustered, or does that not
> > occur in reality?
> >
> > AFAIU the old code managed OM storage itself, used global data
> > structures to do so, and guarded access with a mutex. To reduce
> > contention, it used a surprisingly large thread-local freelist of 1024
> > entries. This looks like contention was once a real problem.
>
> You can always create a benchmark to show contention in the monitor
> inflation code. I don't recall now whether this was a real issue or a
> microbenchmark issue. As the code stated:
>
> ObjectMonitor * ATTR ObjectSynchronizer::omAlloc (Thread * Self) {
>      // A large MAXPRIVATE value reduces both list lock contention
>      // and list coherency traffic, but also tends to increase the
>      // number of objectMonitors in circulation as well as the STW
>      // scavenge costs.  As usual, we lean toward time in space-time
>      // tradeoffs.
>
>      const int MAXPRIVATE = 1024 ;
>
> so general performance was a consideration.
>
> > OTOH the new code just uses malloc, which also may lock depending on the
> > malloc allocator internals and the used libc settings. Therefore I
> > wonder whether OM allocation is still a problem, not a problem with
> > real-life malloc, or maybe never really had been a problem and the old
> > code was just overly cautious?
>
> Whenever we make significant changes to a subsystem we always
> investigate the performance profile of the changes. We're prepared to
> accept some performance loss if we have a good improvement in code
> complexity/maintainability etc, but if a significant performance issue
> arose we would revisit it. See for example discussion in:
>
> https://bugs.openjdk.java.net/browse/JDK-8263864
>
> and related.
>
> Cheers,
> David
> -----
>
> > Thanks, Thomas
> >
> >
>