Re: RFR: 8248238: Implementation: JEP 388: Windows AArch64 Support [v12]

2020-10-04 Thread Monica Beckwith
On Mon, 5 Oct 2020 03:20:43 GMT, David Holmes  wrote:

> @mo-beck The initial comment still has this incorrect link:
> 
> [2] https://openjdk.java.net/jeps/8251280
> 
> Please edit the comment and fix the link.

That was a link to the macOS + Arm64 port. But I have removed it as it wasn't 
needed in the description of this
implementation.

-

PR: https://git.openjdk.java.net/jdk/pull/212


Integrated: 8248238: Implementation: JEP 388: Windows AArch64 Support

2020-10-04 Thread Monica Beckwith
On Wed, 16 Sep 2020 20:26:10 GMT, Monica Beckwith  wrote:

> This is a continuation of 
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>  
> Changes since then:
> * We've improved the write barrier as suggested by Andrew [1]
> * The define-guards around R18 have been changed to `R18_RESERVED`. This will 
> be enabled for Windows only for now but
>   will be required for the upcoming macOS+Aarch64 [2] port as well.
> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov 
> in our PR for now and built the
>   Windows-specific CPU feature detection on top of it.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html
> [2] https://openjdk.java.net/jeps/8251280

This pull request has now been integrated.

Changeset: 9604ee82
Author:Monica Beckwith 
Committer: David Holmes 
URL:   https://git.openjdk.java.net/jdk/commit/9604ee82
Stats: 2566 lines in 62 files changed: 2208 ins; 126 del; 232 mod

8248238: Implementation: JEP 388: Windows AArch64 Support

Co-authored-by: Monica Beckwith 
Co-authored-by: Ludovic Henry 
Co-authored-by: Bernhard Urban-Forster 
Reviewed-by: dholmes, cjplummer, aph, ihse

-

PR: https://git.openjdk.java.net/jdk/pull/212


Re: RFR: 8248238: Implementation: JEP 388: Windows AArch64 Support [v12]

2020-10-04 Thread David Holmes
On Fri, 2 Oct 2020 17:16:50 GMT, Monica Beckwith  wrote:

>> Marked as reviewed by dholmes (Reviewer).
>
>> The JEP is not yet targeted so we have to wait for that formality. But
>> once that happens I can sponsor for you.
> 
> Thanks.
>>
>> Also note that the PR references the wrong JEP so can you please edit
>> the description to fix that.
> 
> Added JEP # (388) here and updated the JBS entry.
> After looking at JEPs 386, 377, and 379, I also did the following:
> - listed JDK-8248238 as a sub-task for JDK-8248496
> - added this PR link in a comment for the JEP.
> 
> As soon as the JEP is targetted, I will update the "Fix version" for the 
> 'Implementation' (JDK-8248238) and ping you
> @dholmes-ora .
>> 
>> Meanwhile I'll see if I can take this for a spin through our internal
>> testing.
> Thanks so much.
> 
> Regards,
> Monica
> 
>> 
>> Cheers,
>> David

@mo-beck The initial comment still has this incorrect link:

[2] https://openjdk.java.net/jeps/8251280

Please edit the comment and fix the link.

-

PR: https://git.openjdk.java.net/jdk/pull/212


Re: RFR: 8245194: Unix domain socket channel implementation [v14]

2020-10-04 Thread Michael McMahon
> Continuing this review as a PR on github with the comments below 
> incorporated. I expect there will be a few more
> iterations before integrating.
> On 06/09/2020 19:47, Alan Bateman wrote:
>> On 26/08/2020 15:24, Michael McMahon wrote:
>>>
>>> As I mentioned the other day, I wasn't able to use the suggested method on 
>>> Windows which returns an absolute path. So,
>>> I added a method to WindowsPath which returns the path in the expected 
>>> UTF-8 encoding as a byte[]. Let me know what you
>>> think of that.
>>>
>>> There is a fair bit of other refactoring and simplification done also. Next 
>>> version is at:
>>>
>>> http://cr.openjdk.java.net/~michaelm/8245194/impl.webrev/webrev.9/
>>>
>> Adding a method to WindowsPath to encode the path using UTF-8 is okay but I 
>> don't think we should be caching it as the
>> encoding for sun_path is an outlier on Windows. I'm also a bit dubious about 
>> encoding a relative path when the resolved
>> path (before encoding) is > 247 chars. The documentation on the MS site 
>> isn't very completely and I think there are a
>> number points that require clarification to fully understand how this will 
>> work with relative, directly relative and
>> drive relative paths.
>>
> 
> Maybe it would be better to just use the path returned from toString() and do 
> the conversion to UTF-8 externally. That
> would leave WindowsPath unchanged.
>> In the same area, the new PathUtil is a bit inconsistent with the existing 
>> provider code. One suggestion is to add a
>> method to AbstractFileSystemProvider instead. That is the base class the 
>> platform providers and would be a better place
>> to get the file path in bytes.
>>
> 
> Okay, I gave the method a name that is specific to Unix domain sockets 
> because this UTF-8 strangeness is not likely to
> be useful by other components.
>> One other comment on the changes to the file system provider it should be 
>> okay to change UnixUserPrinipals ad its
>> fromUid and fromGid method to be public. This would mean that the patch 
>> shouldn't need to add UnixUserGroup (the main
>> issue is that class is that it means we end up with two classes with static 
>> factory methods doing the same thing).
> 
> Okay, that does simplify it a bit.
> 
> Thanks,
> Michael.
> 
>> -Alan.
>>
>>
>>
>>
>>
>>

Michael McMahon has updated the pull request incrementally with one additional 
commit since the last revision:

  - simplified Copy.gmk to CAT source files directly
  - renamed net.properties source files to all be net.properties

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/52/files
  - new: https://git.openjdk.java.net/jdk/pull/52/files/dc70cd6e..0096645e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=52=13
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=52=12-13

  Stats: 9 lines in 4 files changed: 0 ins; 4 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/52.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/52/head:pull/52

PR: https://git.openjdk.java.net/jdk/pull/52


Re: RFR: 8245194: Unix domain socket channel implementation [v13]

2020-10-04 Thread Michael McMahon
On Sun, 4 Oct 2020 08:27:39 GMT, Alan Bateman  wrote:

>> Good points. I will update as suggested. Thanks.
>
> I would prefer if we didn't rename net.properties. Can we use the same 
> approach as lib/security/default.policy where
> the share and platform specific are concatenated?

Okay, I have just pushed these suggested updates.

-

PR: https://git.openjdk.java.net/jdk/pull/52


Re: RFR: 8245194: Unix domain socket channel implementation [v13]

2020-10-04 Thread Alan Bateman
On Fri, 2 Oct 2020 13:17:04 GMT, Michael McMahon  wrote:

>> make/modules/java.base/Copy.gmk line 195:
>> 
>>> 193:$(call MakeTargetDir)
>>> 194:$(RM) $@ $@.tmp
>>> 195:$(foreach f,$(NET_PROPERTIES_SRC_LIST),$(CAT) $(f) >> $@.tmp;)
>> 
>> This can be simplified. Cat takes multiple files as input, so no need for 
>> 'foreach'. Also no need to go via a temp
>> file. We have make configured to delete targets if a recipe fails, so the 
>> tmp dance isn't needed. (I know we still have
>> this pattern all over the place, but we are trying to quit the practice)
>
> Good points. I will update as suggested. Thanks.

I would prefer if we didn't rename net.properties. Can we use the same approach 
as lib/security/default.policy where
the share and platform specific are concatenated?

-

PR: https://git.openjdk.java.net/jdk/pull/52