RFR: 8036128 Remove deprecated VM flag UseVMInterruptibleIO

[frederic parain]

Karen,

Thank you for the review and all your investigations
on the Solaris connect() semantic.

Fred

On 13/03/2014 17:31, Karen Kinnear wrote:
> 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
>

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