RFR[13]: 8226366: Excessive ServiceThread wakeups for OopStorage cleanup

[Kim Barrett]

> On Jul 1, 2019, at 5:36 PM, coleen.phillimore at oracle.com wrote:
> 
> 
> http://cr.openjdk.java.net/~kbarrett/8226366/open.00/src/hotspot/share/runtime/serviceThread.cpp.frames.html
> 
> Do you have another bug to add the oopStorage for the ResolvedMethodTable to the list?

JDK-8227053.  Also JDK-8227054.

> http://cr.openjdk.java.net/~kbarrett/8226366/open.00/src/hotspot/share/runtime/safepoint.cpp.frames.html
> 
> I suppose you don't need is_safepoint_needed() to trigger this cleanup in the GuaranteedSafepointInterval because if there is no GC, there won't be any blocks to deallocate.

s/is_safepoint_needed()/is_cleanup_needed()/

There doesn't seem to be a clear theory of what that function should
check for. Some of the existing safepoint cleanups have checks there,
some don't, and it's not always obvious why. This cleanup doesn't seem
so urgent that if there are no other reasons to safepoint for a
long(ish) time then we should force one for just this purpose.

> http://cr.openjdk.java.net/~kbarrett/8226366/open.00/src/hotspot/share/gc/shared/oopStorage.cpp.frames.html
> 
> One nit.  The rest of the implementations that do the same thing as this, are called "trigger_concurrent_work".  This is called differently from the safepoint cleanup tasls, but could you call it trigger_cleanup_if_needed() instead?  Then I know it does the same/similar thing as the others without looking.

Renamed to trigger_cleanup_if_needed().

Also test_and_clear_cleanup_request() => has_cleanup_work_and_reset().
Thomas asked for "has_work", to be consistent with
String/Symbol/ResolvedMethodTable, but I think that's too generic here;
what kind of work? (In the case of the tables, it's not always
"cleanup" work.) Coleen suggested the "and_reset" suffix, to follow an
existing convention.

I also made a few corresponding internal name changes.

> The implementation looks good.  I think it's good that you don't have the safepoint cleanup task timer around this.

I added a comment about the lack of task timing, so it's clearly
intentional and not simply forgotten.

New webrevs:
full: http://cr.openjdk.java.net/~kbarrett/8226366/open.01/
incr: http://cr.openjdk.java.net/~kbarrett/8226366/open.01.inc/

Testing: Local build and hotspot_tier1.