RFR (s): 8251460: Fix the biased-locking code in ObjectSynchronizer::FastHashCode
David Holmes
david.holmes at oracle.com
Wed Aug 12 22:17:09 UTC 2020
Hi Dan,
On 13/08/2020 1:26 am, Daniel D. Daugherty wrote:
>> webrev: http://cr.openjdk.java.net/~dholmes/8251460/webrev/
>
> src/hotspot/share/runtime/synchronizer.cpp
> No comments.
>
> Thumbs up!
Thanks for the review.
> Thanks for the info about how this was tested. Any Mach5 testing
> done yet?
Putting it through t1-3 now. Will push once that passes.
Thanks,
David
-----
> Dan
>
>
> On 8/12/20 8:16 AM, David Holmes wrote:
>> Thanks Coleen!
>>
>> I forgot to mention that the testing for this was to reenable (in
>> arguments.cpp) biased-locking for CDS (and fix an errant assert that
>> Ioi is going to fix) and run the CDS tests. None of those changes will
>> be pushed of course.
>>
>> David
>>
>> On 12/08/2020 9:58 pm, Coleen Phillimore wrote:
>>> Looks good!
>>> Coleen
>>>
>>> On 8/12/20 12:23 AM, David Holmes wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8251460
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8251460/webrev/
>>>>
>>>> During the review for JDK-8250606 it was noted that the
>>>> biased-locking section of FastHashCode contained a similar assertion
>>>> regarding execution at a safepoint:
>>>>
>>>> // Relaxing assertion for bug 6320749.
>>>> assert(Universe::verify_in_progress() ||
>>>> !SafepointSynchronize::is_at_safepoint(),
>>>> "biases should not be seen by VM thread here");
>>>>
>>>> which gives the impression that it is okay to execute this code
>>>> block at a safepoint if verify_in_progress() is true. But if we
>>>> examine the next line of code:
>>>>
>>>> BiasedLocking::revoke(hobj, JavaThread::current());
>>>>
>>>> we find that at a safepoint:
>>>> a) JavaThread::current() will assert because the current thread will
>>>> be the VMThread; and
>>>> b) BiasedLocking::revoke itself contains an assertion that it is not
>>>> invoked at a safepoint.
>>>>
>>>> In practice there is no issue with revoking the bias at a safepoint,
>>>> we just have to use the right code to do so (as done elsewhere in
>>>> the same file):
>>>>
>>>> if (SafepointSynchronize::is_at_safepoint()) {
>>>> BiasedLocking::revoke_at_safepoint(hobj);
>>>> } else {
>>>> BiasedLocking::revoke(hobj, self);
>>>> }
>>>>
>>>> Thanks,
>>>> David
>>>
>
More information about the hotspot-runtime-dev
mailing list