RFR: 8036128 Remove deprecated VM flag UseVMInterruptibleIO

Thu Mar 13 16:31:56 UTC 2014

Frederic,

The updated webrev looks great - and now that Michael McMahon has retested Solaris 10
to find out that we still have this behavior - we still need the os::connect changes.

Could you add one comment to that code please, perhaps the one currently
in PlainSocketImp.c - which references the 6343810 bug and the fact that this
occurs for blocking sockets for Solaris 10 still. I don't need to re-review.

Based on your testing and Staffan's comments, sounds like we are good with jstat -
since we didn't document how to extend your options.

If anyone wants to know how to do that, I have a description and a sample file.

So - all set - and many thanks,
Karen

On Mar 10, 2014, at 11:25 AM, frederic parain wrote:

> 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.
>> 
>> 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