RFR (M) 8186042: Optimize OopMapCache lookup

[coleen.phillimore at oracle.com]

On 8/27/17 11:37 PM, David Holmes wrote:
> Hi Coleen,
>
> On 26/08/2017 12:05 AM, coleen.phillimore at oracle.com wrote:
>>
>> This was reviewed on hotspot-runtime-dev but it was noticed that the 
>> traversal of the interpreter's OopMapCache can be done outside of a 
>> safepoint by the graal compiler and by stack walking.   This would 
>> make reclamation of the "old" cached entries not safe.  I have 
>> disallowed use of the interpreter OopMapCache in this change by any 
>> calls outside of GC.   The OopMapCache is an optimization primarily 
>> (now exclusively) for GC root walking.
>
> That seems a very reasonable restriction. This looks good to me.
>
> One nit: src/share/vm/oops/method.cpp
>
> +  // and other uses generate an oopmap but do not save it in the cache.
>
> and -> any?

Thank you,  I'll fix that typo.

thanks!
Coleen
>
> Thanks,
> David
> -----
>
>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8186042.02/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8186042
>>
>> Retested with jck tests with -Xlog:interpreter+oopmap=debug, hotspot 
>> tier1 which includes the compiler AOT tests that failed, jvmti tests, 
>> and stack walking tests in the jdk.
>>
>> Thanks,
>> Coleen
>>
>> On 8/11/17 11: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
>>>
>>> Tested with RBT equivalent of nightly on linux x64.  Also ran dacapo 
>>> with -Xint -Xlog:interpreter+oopmap=debug to verify. This change 
>>> also removes -XX:+TraceOopMapGeneration (not 
>>> -XX:+TraceNewOopMapGeneration however) in favor of new logging.  A 
>>> linked CSR request is pending.
>>>
>>> Thanks,
>>> Coleen
>>>
>>