RFR(s): 8234086: VM operation can be simplified

[Kim Barrett]

> On Nov 21, 2019, at 3:00 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
> On 2019-11-20 21:08, Kim Barrett wrote:
>>> On Nov 20, 2019, at 4:38 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
>>> On 11/19/19 4:55 PM, Kim Barrett wrote:
>>>> src/hotspot/share/runtime/synchronizer.cpp
>>>> 1006   if (Atomic::load(&ForceMonitorScavenge) == 0 && Atomic::xchg (1, &ForceMonitorScavenge) == 0) {
>>>> 1007     VMThread::check_cleanup();
>>>> I was going to make a couple nit-pick comments here, but this seems to
>>>> be soon to be dead code (only reached when deprecated MonitorBound has
>>>> a non-default value).
>>>> Not sure it's worth changing this and adding check_cleanup just for
>>>> use here though, given this code is going away soon. Are there planned
>>>> future uses of check_cleanup?
>>> 
>>> No future plans, no.
>>> Are you suggesting doing a synchronous safepoint here instead?
>> I think I (and maybe David too) need to better understand what is being done here and why.
> 
> Sorry for not making that clear, we have this comment explaining the situation:
> 
>      const int mx = MonitorBound;
>      if (mx > 0 && (g_om_population-g_om_free_count) > mx) {
>        // Not enough ObjectMonitors on the global free list.
>        // We can't safely induce a STW safepoint from om_alloc() as our thread
>        // state may not be appropriate for such activities and callers may hold
>        // naked oops, so instead we defer the action.
>        InduceScavenge(self, "om_alloc");
>      }

I see; this is a place where we were using the async VMOp feature and
still (according to the comment) can't do a synchronous safepoint
here.  So a bit of somewhat new code to cope with this.  And since we
only get here when MonitorBound is exceeded, and that's already
deprecated (JDK-8230938) and slated for removal in jdk15, that new
code will be going away soonish.  But you want to make progress on
this change now, rather than waiting for that "soonish".  OK.

> I'm not sure why we only do this if we try to take a monitor from global free list. If free list is empty we just allocate instead.

No idea; seems like while the global free list is empty we can just continue
allocating, without regard to MonitorBound.  Though presumably we’ll
eventually safepoint and check for cleanup using the other mechanism.