RFR: 8210986: Add OopStorage cleanup to ServiceThread

Robbin Ehn robbin.ehn at oracle.com
Mon Nov 5 09:56:54 UTC 2018


Hi Kim,

Seems reasonable, I must admit I hate the MutexUnlocker, but I see way you 
choose to that pattern here.

It's not obvious to me why you needed to change the:
644   OrderAccess::storeload();
to
668   OrderAccess::fence();

Thanks, Robbin


On 10/19/18 10: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.
> 


More information about the hotspot-dev mailing list