On Wed, 11 Nov 2020 04:50:52 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> resolve more robehn and coleenp comments.
>
> src/hotspot/share/runtime/synchronizer.cpp line 153:
>
>> 151: if (self->is_Java_thread()) {
>> 152: // A JavaThread must check for a safepoint/handshake and honor it.
>> 153: ObjectSynchronizer::chk_for_block_req(self->as_Java_thread(),
>> "unlinking",
>
> I won't disapprove but this is a case where refactoring is IMO worse than
> code duplication. Logging parameters should not be a part of this API IMO.
At this point, the refactoring has grown on me. I like the fact that
it reduces the "noise" in those three functions.
> src/hotspot/share/runtime/synchronizer.cpp line 1228:
>
>> 1226: os::naked_short_sleep(999); // sleep for almost 1 second
>> 1227: } else {
>> 1228: os::naked_short_sleep(999); // sleep for almost 1 second
>
> So this block can now just be:
>
>> if (self->is_Java_thread()) {
>> ThreadBlockInVM tbivm(self->as_Java_thread());
>> }
>> os::naked_short_sleep(999); // sleep for almost 1 second
As @robehn pointed out, I screwed up that change because
I lost the block over duration of the sleep. I've reverted that
change to the original.
-------------
PR: https://git.openjdk.java.net/jdk/pull/642
