RFR: 8000975: (process) Merge UNIXProcess.java.bsd & UNIXProcess.java.linux (& .solaris & .aix)

[Peter Levart]

On 04/01/2014 05:43 PM, Peter Levart wrote:
> On 04/01/2014 03:49 PM, roger riggs wrote:
>> Hi Peter,
>>
>> The design using enum for the os dependencies does not make it possible
>> to include only the support needed for a particular platform at build 
>> time.
>> Every implementation will be carrying around the support for all the 
>> other platforms.
>> A build time binding would be more efficient.
>>
>> Roger
>
> That's true. A trade-off between maintainability and efficiency. The 
> efficiency has two categories here. One is the size of the 
> distributable and the other is run-time efficiency. I've been thinking 
> to improve both efficiencies (the run-time in particular) with a 
> little re-design. Since nearly each OS platform requires a sub-class 
> of UNIXProcess to implement the differences, I can move the 
> implementations of various methods now in Os enum to the UNIXProcess 
> subclasses and get rid of Os enum per-instance subclasses.
>
> Let me try this and see what comes out.

Hi Roger,

Well, it turns out the methods would like to stay in Os (renamed to 
Platform), but there is no need for per-enum-instance subclasses. Using 
enum constructor parameters and switch statements makes code even more 
compact and easy to follow...

http://cr.openjdk.java.net/~plevart/jdk9-dev/UNIXProcess/webrev.04/


I belive there is still room for consolidating logic in various 
Input/OutputStream wrappers used in UNIXProcess variants. But in the 
first round I tried to preserve the exact behaviour. If the wrapping of 
streams could be made more-or-less equal in all UNIX platforms, then the 
need for UNIXProcess subclasses and/or overhead of support classes 
included but not used goes away...

Regards, Peter

>
>>
>>
>>
>> On 4/1/2014 9:16 AM, Peter Levart wrote:
>>> Hi Alan, Volker,
>>>
>>> Thanks for sharing the info and for testing on AIX. Here's the 
>>> updated webrev that hopefully includes the correct "dispatch on 
>>> os.name" logic. I included "Solaris" as an alternative to "SunOS" 
>>> since I saw this in some documents on Internet. If this is 
>>> superfluous, I can remove it:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/UNIXProcess/webrev.03/
>>>
>>> I tested this on Linux and Solaris and the java/lang/ProcessBuilder 
>>> jtreg tests pass. So with Volker's test on AIX, the only OS platform 
>>> left for testing is Mac OS X. Would someone volunteer?
>>>
>>> Regards, Peter
>>>
>>> On 03/27/2014 11:18 AM, Volker Simonis wrote:
>>>> Hi Peter,
>>>>
>>>> thanks for applying these changes to the AIX files as well.
>>>>
>>>> With the additional line:
>>>>
>>>>              if (osName.equals("AIX")) { return AIX; }
>>>>
>>>> in Os.get() your change compiles cleanly on AIX and runs the
>>>> java/lang/ProcessBuilder tests without any errors.
>>>>
>>>> So from an AIX perspective, thumbs up.
>>>>
>>>> Regards,
>>>> Volker
>>>>
>>>>
>>>> On Wed, Mar 26, 2014 at 5:18 PM, Alan Bateman 
>>>> <Alan.Bateman at oracle.com> wrote:
>>>>> On 26/03/2014 15:19, Peter Levart wrote:
>>>>>> I couldn't find any official document about possible os.name 
>>>>>> values for
>>>>>> different supported OSes. Does anyone have a pointer?
>>>>> I don't know if there is a definite list but I assume we don't 
>>>>> need to be
>>>>> concerned with anything beyond the 4 that we have in OpenJDK, 
>>>>> which is
>>>>> "Linux", "SunOS", "AIX" and contains("OS X").
>>>>>
>>>>> If we get to the point in JDK 9 where src/solaris is renamed to 
>>>>> src/unix (or
>>>>> something equivalent) then it could mean that the Os enum can be 
>>>>> replaced
>>>>> with an OS specific class in src/linux, src/solaris, ... and this 
>>>>> would
>>>>> avoid the need for an os.name check at runtime.
>>>>>
>>>>> -Alan.
>>>>>
>>>
>>
>