RFR: JDK-8043630 Method os::yield_all() should be removed

[frederic parain]

David,

Thank you for reviewing, my answers are
in-lined below.

On 22/05/2014 14:07, David Holmes wrote:
> Hi Fred,
>
> Generally good to see this go but a couple of queries.
>
> src/share/vm/prims/jni.cpp
>
> You changed yield_all to yield but on Solaris (actually posix) that
> still introduces the thread-state-transition so the ThreadInVMfromNative
> would still seem to be needed.

Good catch! I should have replaced yield_all() with NakedYield()
instead of yield(). Fixed and webrev updated:

http://cr.openjdk.java.net/~fparain/8043630/webrev.01/

Note: I've launched a new JPRT job to test jdk_nio test suite.

> My memory is failing me here as I don't recall the details of the switch
> to using os::posix::sleep on Solaris. But I note that in doing so we
> have lost the semantics of sleep(0) that os::yield was relying on - we
> lost the thr_yield call!

The Solaris implementation of sleep() has been merged with other
Posix implementation to fix CR 6546236 (the race condition between
Thread.interrupt() and Thread.sleep()).

> src/share/vm/services/memTracker.hpp
>
> Aside: I'm curious to know why NakedYield does not work as well on
> Windows - does it yield to the "wrong" thread?

Zhengyu already answered this one.

Thanks,

Fred

> On 22/05/2014 9:22 PM, frederic parain wrote:
>> Please review the following change to remove
>> the os::yield_all() method. This method has
>> been source of issues for a long time and it's
>> time to get rid of it.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8043630
>>
>> Webrev:
>> http://cr.openjdk.java.net/~fparain/8043630/webrev.00/
>>
>> This changeset has been tested with JPRT (builds and
>> tests), vm.quick.testlist, JDK jdk_core.
>> I also ran refworkload benchmarks suite which didn't
>> show any significant regression.
>>
>> Thanks,
>>
>> Fred
>>

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at oracle.com