RFR (S): 8230423: Move os::sleep to JavaThread::sleep

[David Holmes]

Hi Dan,

Are you okay with this?

Thanks,
David

On 10/09/2019 7:41 am, David Holmes wrote:
> Hi Dan,
> 
> Thanks for taking a look.
> 
> On 10/09/2019 3:27 am, Daniel D. Daugherty wrote:
>> On 9/9/19 1:21 AM, David Holmes wrote:
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8230423
>>> webrev: http://cr.openjdk.java.net/~dholmes/8230423/webrev/
>>
>> src/hotspot/cpu/x86/rdtsc_x86.cpp
>>      No comments.
>>
>> src/hotspot/os/posix/os_posix.cpp
>>      L648:   assert(thread->is_Java_thread(), "invariant");
>>      L649:   JavaThread* jt = (JavaThread*) thread;
>>          I was expecting this change with your "interrupt" bug.
>>          Are we ready for this assert() now?
>>
>> src/hotspot/os/windows/os_windows.cpp
>>      L3600:   assert(thread->is_Java_thread(), "invariant");
>>      L3601:   JavaThread* jt = (JavaThread*) thread;
>>          I was expecting this change with your "interrupt" bug.
>>          Are we ready for this assert() now?
> 
> It is already true that only JavaThreads use the interrupt facility. The 
> forthcoming changes to the interrupt just make that more obvious:
> 
> os::interrupt is-called-from
>    Thread::interrupt is called from
>      JVM_Interrupt  // java.lang.Thread interrupt
>      JvmtiEnv::InterruptThread // JVM TI InterruptThread
>      JavaThread::send_thread_stop
> 
> so always applied to a JavaThread.
> 
>> src/hotspot/share/gc/shenandoah/shenandoahPacer.cpp
>>      No comments.
>>
>> src/hotspot/share/jvmci/jvmciCompiler.cpp
>>      No comments.
>>
>> src/hotspot/share/jvmci/jvmciRuntime.cpp
>>      No comments.
>>
>> src/hotspot/share/prims/jvm.cpp
>>      No comments.
>>
>> src/hotspot/share/runtime/os.cpp
>>      No comments.
>>
>> src/hotspot/share/runtime/os.hpp
>>      No comments.
>>
>> src/hotspot/share/runtime/thread.cpp
>>      L1701:   _SleepEvent  = ParkEvent::Allocate(this);
>>      L1815:   _SleepEvent  = NULL;
>>          nit - please delete extra blank before '='.
>>
>>      L3403 - nit - please delete extra blank line.
> 
> Nits will be fixed.
> 
> Thanks,
> David
> -----
> 
>> src/hotspot/share/runtime/thread.hpp
>>      No comments.
>>
>> test/hotspot/gtest/gc/g1/test_g1FreeIdSet.cpp
>>      No comments.
>>
>> test/hotspot/gtest/gc/shared/test_ptrQueueBufferAllocator.cpp
>>      No comments.
>>
>> test/hotspot/gtest/utilities/test_singleWriterSynchronizer.cpp
>>      No comments.
>>
>>
>> Thumbs up! I don't need to see a new webrev if you decide to
>> fix the nits.
>>
>> Dan
>>
>>
>>>
>>> This is the next step in the sleep/interrupt support code reshuffle. 
>>> Now that os::sleep is platform independent and only applicable to 
>>> JavaThreads, we can move it to JavaThread as an instance method.
>>>
>>> Summary of changes:
>>> - os::sleep moved to JavaThread::sleep as instance method
>>> - signature changed to return bool - true means timed-out; false 
>>> means interrupted
>>> - rearranged the sleep code slightly to remove the initial 
>>> back-to-back nanoTime() calls - as suggested by Roger.
>>> - _SleepEvent moved from Thread to JavaThread
>>> - minor changes to os::interrupt to account for move to JavaThread 
>>> (this code will also be moved to JavaThread in the next fix)
>>> - call sites changed from os::sleep(t,ms) to t->sleep(ms)
>>>
>>> Testing:
>>>  - tiers 1-3
>>>
>>> Thanks,
>>> David
>>