RFR:8170868: DefaultProxySelector should use system defaults on Windows, MacOS and Gnome

[Volker Simonis]

Hi Chris,

this change is fine for me now and I think Arno addressed all of your
concerns. Is it OK for you if I push this to 9 and add you as one of
the reviewers?

Thank you and best regards,
Volker


On Wed, Jan 11, 2017 at 3:21 PM, Zeller, Arno <arno.zeller at sap.com> wrote:
> Hi Chris,
>
> I have addressed your comments in a new webrev. Can you please have a look at this one?
>
> http://cr.openjdk.java.net/~clanger/webrevs/8170868.3/
>
> I created src/java.base/share/native/libnet/proxy_util.c/h and these files contain now the common used native parts.
> And I rewrote the code to return an array of Proxy objects from native code - so I do no longer call back to Java to
> add an object to the list.
>
> Best regards,
> Arno
>
>>-----Original Message-----
>>From: Chris Hegarty [mailto:chris.hegarty at oracle.com]
>>Sent: Dienstag, 3. Januar 2017 15:04
>>To: Zeller, Arno <arno.zeller at sap.com>
>>Cc: net-dev at openjdk.java.net
>>Subject: Re: RFR:8170868: DefaultProxySelector should use system defaults
>>on Windows, MacOS and Gnome
>>
>>Arno,
>>
>>> On 27 Dec 2016, at 11:44, Zeller, Arno <arno.zeller at sap.com> wrote:
>>>
>>> Hi Chris,
>>>
>>> Thanks for having a look at my change:
>>>
>>>> 1) It seems awful to have to deal with LinkedList in native code. How
>>>>   about returning an array from native, and then converting that into
>>>>   whatever list type is appropriate at the Java level.
>>> With the current implementation on Mac I do not know upfront how many
>>items the list will contain and therefore I preferred to use the LinkedList from
>>native code.
>>> I can of course change the implementation on Mac to first generate a fully
>>resolved native list and then I can use NewObjectArray to generate an Array
>>of Proxy objects to return. This will avoid calling to java to add an element to
>>the list. Do you think this will be better?
>>
>>Yes. Thanks.
>>
>>>> 2) I would prefer the use of List.of(...), and list.of() for empty, since
>>>>   these are immutable and efficient list implementations.
>>> I thought about that but I tried to not return an immutable List because the
>>Javadoc of Proxy.select does not state anything about the returned List (if it is
>>immutable or not) and I feared to break compatibility by returning an
>>immutable List.
>>> If you think this is no problem I will of course prefer to always return the
>>same immutable NO_PROXY list object.
>>
>>I would prefer an immutable list. Maybe we file, a separate, issue to amend
>>the spec to say "returns an immutable list …”.
>>
>>>> 3) Is the comment “Inspired ...” necessary / appropriate?
>>> I will change it to the comment proposed by Volker.
>>
>>Ok.
>>
>>>> 4) Can some of the native initialization code be moved to a platform
>>>>   independent location, to remove duplication?
>>> Would it be ok if I move the definition of the static variables and the
>>implementation of 'static int initJavaClass(...)' to a header file in the shared
>>branch. I.e. src/java.base/shared/native/libnet/DefaultProxySelector.h and
>>include this in the MacOSX, Windows and the Unix implementations?
>>
>>Yes.
>>
>>>> 5) The new file has a shared copyright header. I see similar SAP
>>>>   headers in a few files, but none shared with the Oracle header.
>>>>   How did you arrive at this format?
>>> The format was suggested to me by Volker :-)
>>
>>Ok.
>>
>>-Chris.