RFR (M) 8186042: Optimize OopMapCache lookup

David Holmes david.holmes at oracle.com
Mon Aug 14 00:29:38 UTC 2017


Hi Coleen,

This looks good to me - Reviewed. A couple of minor comments below.

On 12/08/2017 1:46 AM, coleen.phillimore at oracle.com wrote:
> Summary: Use lock free access to oopMapCache
> Contributed-by: frederic.parain at oracle.com, coleen.phillimore at oracle.com
> 
> The OopMapCache::lookup() function took out a mutex to protect access 
> between the GC threads that are running concurrently.  See bug for more 
> info.   The function lookup() is run by multiple GC threads 
> concurrently.  If there's a collision in the hashtable, this uses atomic 
> cmpxchg to add the entry to a list to be cleaned up after the safepoint 
> is over.   GC isn't doing lookup at that point.
> 
> This change is contributed by Frederic Parain, with some cleanup and 
> logging from me.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8186042.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8186042

src/share/vm/interpreter/oopMapCache.cpp

The logging here doesn't follow our usual pattern of only making the 
logging calls if the appropriate logging level is enabled. ??

src/share/vm/interpreter/oopMapCache.hpp

Nit: OopMapCacheEntry* volatile* _array;

Space after volatile please as it refers to the OMCE* not the *_array.

> Tested with RBT equivalent of nightly on linux x64.  Also ran dacapo 
> with -Xint -Xlog:interpreter+oopmap=debug to verify.  This change also 

Do you have some performance improvement numbers for this optimization? 
Or is this a pre-emptive strike against a potential locking bottleneck?

> removes -XX:+TraceOopMapGeneration (not -XX:+TraceNewOopMapGeneration 
> however) in favor of new logging.  A linked CSR request is pending.

Ok.

Thanks,
David

> Thanks,
> Coleen
> 


More information about the hotspot-runtime-dev mailing list