RFR: 8036128 Remove deprecated VM flag UseVMInterruptibleIO

[frederic parain]

Karen,

my answers are in-lined below.

On 07/03/2014 20:02, Karen Kinnear wrote:
> Frederic,
>
> 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:

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

> 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
page:

http://docs.oracle.com/javase/7/docs/technotes/tools/share/jstat.html

Thanks,

Fred

> 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.
>
> thanks,
> Karen
>
> On Mar 6, 2014, at 11:40 PM, David Holmes wrote:
>
>> Hi Fred,
>>
>> This looks good to me! Great to see the cleanup happen.
>>
>> os_solaris.cpp:
>>
>> 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. ;-)
>>
>> Cheers,
>> David
>>
>> On 7/03/2014 2:12 AM, frederic parain wrote:
>>> Greetings,
>>>
>>> The UseVMInterruptibleIO flag removal has been
>>> scheduled a long time ago:
>>> https://bugs.openjdk.java.net/browse/JDK-4385444
>>>
>>> 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:
>>> https://bugs.openjdk.java.net/browse/JDK-8036128
>>>
>>> The webrev:
>>> http://cr.openjdk.java.net/~fparain/8036128/webrev.00/
>>>
>>> Tested with vm.quick.testlist and JPRT hotspot job.
>>>
>>> Thanks,
>>>
>>> Fred
>>>
>

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