RFR: 8210986: Add OopStorage cleanup to ServiceThread

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~kbarrett/8210986/open.00/src/hotspot/share/gc/shared/oopStorage.cpp.frames.html

392 // it is called in all kinds of contexts where even quote low ranked 
locks

typo: s/quote/quite/

425 Block* block = block_for_allocation();
426 if (block == NULL) return NULL; // Block allocation failed.

This could be "get_block_for_allocation" because it's not blocking 
(afaict).  Name is somewhat ambiguous.

http://cr.openjdk.java.net/~kbarrett/8210986/open.00/src/hotspot/share/runtime/serviceThread.cpp.frames.html

ick.  Since this is a static list, can it be declared outside the scope 
of ServiceThread::entry(), like before the functions that loop through 
the oopstorages?

109 OopStorage* const oopstorages[] = {
110 JNIHandles::global_handles(),
111 JNIHandles::weak_global_handles(),
112 StringTable::weak_storage(),
113 SystemDictionary::vm_weak_oop_storage()
114 };
115 const size_t oopstorage_count = ARRAY_SIZE(oopstorages);
116

I think it would bother me less that we have to have all the OopStorages 
listed somewhere, if it were static scope and not in the service thread 
function.  Are these all the ones we have?

Otherwise I think this is fine.

Thanks,
Coleen

On 10/19/18 4:47 PM, Kim Barrett wrote:
> Please review this change to the Service thread to delete empty
> OopStorage blocks when needed.
>
> As part of this, eliminated delete_empty_blocks_safepoint, and renamed
> delete_empty_blocks_concurrent to remove the now redundant _concurrent
> suffix.  Also added a predicate for the Service thread's use, to test
> whether there is cleanup work to be done.
>
> The previously unused empty block deletion has been revised for this
> new usage.  This includes making loops safepoint polite.
>
> Various parts of OopStorage are now aware of the Service thread
> cleanup and notify that thread when appropriate.  (allocate's
> obtaining the block to allocate from was refactored to make it easier
> to insert that notification.)
>
> As part of this, OopStorage::Block::release_entries now takes the
> owning storage object as an argument (rather than a pointer to its
> _deferred_updates list).  This allows release_entries to perform
> additional operations on the owner (so long as C++ DR45 has been
> fixed, and we've added more AIX-specific workarounds for that).
>
> While touching the Service thread, fixed Service_lock locking to use
> use MonitorLockerEx rather than MutexLockerEx.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8210986
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8210986/open.00/
>
> Testing:
> Mach5 tier1-5.
> Ran some performance tests to verify no regressions due to additional
> load on the Service thread.
>