RFR: 8036128 Remove deprecated VM flag UseVMInterruptibleIO
frederic.parain at oracle.com
Mon Mar 10 15:25:20 UTC 2014
my answers are in-lined below.
On 07/03/2014 20:02, Karen Kinnear wrote:
> This looks great. And thank you so much.
> Questions for you:
> 1. os::read
> Yes we need to clean this up. Thank you for pointing that out.
> I am concerned that if we skip state transitions when making OS calls that can block, that
> we can possibly increase pause times trying to get to a safepoint, or possibly interfere with
> requests for thread suspension. I updated 6523731 to include this in our cleaning up our
> thread state transitions.
Thanks for updating the CR.
> 2. os::connect
> The change to RESTARTABLE confuses me. I expected to see the current two step handling
> in case of an EINTR (see 6343810 - at the very end it describes why this is Solaris-specific).
> Or are you removing that because this will only be used internally and in debug mode and not
> from the JDK soon?
Thank you for catching that. As far as I understand it, the two steps
handling was there to support non-blocking sockets, which are not used
by JVM code, and of the os::connect() is not used by JDK code anymore.
However, I've restored the initial two-steps handling and added comments
to explain the semantic of the different errno values. The previous
behavior of os::connect() is now preserved.
The new webrev is here:
> 3. jstat
> I didn't see this in your tested list - and I believe that the way this works is dynamic, so you are probably
> ok, but - since you removed perfcounters
> - what happens if your ~/.jvmstat/jstat_options was referencing this data?
> and you run jstat -options and then try to dump these options?
I tested with jcmd (jcmd PerfCounter.print <vmid>) and didn't noticed
anything wrong. However, I don't how to use jvmstat/jstat to read
performance counters. This feature is not documented in jstat web
> David - I asked Frederic to put this flag in the obsolete_jvm_flags - that is our current deprecation mechanism -
> i.e. for all the flags we are deprecating in JDK9, they should have obsoleted_in: 9, accept_until: 10. So the mechanism isn't
> obsolete - it triggers a warning mechanism. We can rename it and rework it to not mention HSX, but we still
> need to deprecate product flags even without HSX. Can we remove really old entries? Yes. Should we keep flags in there
> that reference JDK9? I would prefer to keep those in for JDK9, so the information gets printed for customers about
> when the flag became obsolete.
> On Mar 6, 2014, at 11:40 PM, David Holmes wrote:
>> Hi Fred,
>> This looks good to me! Great to see the cleanup happen.
>> This comment block seems redundant now:
>> 5264 /*
>> 5265 * XXX: is the following call interruptible? If so, this might
>> 5266 * need to go through the INTERRUPT_IO() wrapper as for other
>> 5267 * blocking, interruptible calls in this file.
>> 5268 */
>> Aside: arguments.cpp - I guess we can file an RFE to get rid of obsolete_jvm_flags now that hsx is dead. ;-)
>> On 7/03/2014 2:12 AM, frederic parain wrote:
>>> The UseVMInterruptibleIO flag removal has been
>>> scheduled a long time ago:
>>> Now, it's time to effectively remove this flag and
>>> its associated code.
>>> Removing this feature includes removing all the
>>> macros used to deal with interruptible I/Os, which
>>> could make the reading of the webrev hard and painful.
>>> I conservatively preserved the asserts that were
>>> inserted by the INTERRUPTIBLE macros, with one
>>> notable exception for os::read(). The original
>>> asserts checked that the current ThreadState
>>> was not _thread_in_native nor _thread_blocked.
>>> I changed it into an assert checking that the
>>> current thread state is _thread_in_vm. The
>>> rational for that is that the only real usage
>>> of os::read() on Solaris is in the
>>> ClassPathDirEntry::open_stream() method, which
>>> is always called with ThreadState ==_thread_in_vm.
>>> This change makes the TreadStateTransition simpler
>>> and avoid having to store the previous ThreadState.
>>> This choice could be revisited once the rules
>>> for ThreadStateTransition around system calls
>>> when ThreadState is _thread_in_vm are clarified
>>> (Solaris is currently the only platform doing
>>> this kind of transition for os::read()).
>>> The CR:
>>> The webrev:
>>> Tested with vm.quick.testlist and JPRT hotspot job.
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