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

frederic parain frederic.parain at oracle.com
Thu May 22 13:47:32 UTC 2014


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:


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.



> 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

More information about the hotspot-runtime-dev mailing list