RFR: 8224974: Implement JEP 352

Brian Burkhalter brian.burkhalter at oracle.com
Wed Jun 5 14:36:09 UTC 2019


HI Andrew,

I have some minor comments on webrev.05. (webrev.06 looks rather strange.)

Thanks,

Brian

Note: Hotspot changes not reviewed here.

General
Check copyrights. Some headers are missing altogether, some have incorrect years.

MappedByteBuffer.java
84:	“any of other modes” -> “any of the other modes”, “this” -> “This”
85:	nit: usually use American spelling: “behaviour” -> “behavior” (but it does not matter so much here as it is not public javadoc.
172:	“pasing” -> “passing”
175:	delete “using”
181:	comma or semicolon before “otherwise”
355-365: Collapse to?

	Unsafe.getUnsafe().writebackMemory(address + index, length);

Unsafe.java
924-945:	Capitalize first words of sentences.
959:		Is there a possibility of overflow, e.g., use Math.addExact(address, length) ?
1290:	Insert blank line after.

FileChannelImpl.c (Unix)
85:	“||! map_sync” -> “”|| !map_sync”
98-110:	Put symbolic name definitions at head of file?

FileChannelImpl.c (Windows)
91:	Use same message as at Unix version line 135?

ExtendedMapMode
13:	Constant name should be MAP_MODE_CONSTRUCTOR.
13:	Insert blank line after
34-36:	Move to before line 13.
24:	Move constructor to end of class.

> On Jun 3, 2019, at 7:37 AM, Andrew Dinn <adinn at redhat.com> wrote:
> 
> 
> 
> On 03/06/2019 14:17, Andrew Dinn wrote:
>> Hi Vladimir,
>> 
>> Thanks for the follow-up and for checking the submit failure. I have
>> addressed all the points you raised (point by point comments are
>> included below). A new  webrev which passes a submit run is here:
>> 
>>  http://cr.openjdk.java.net/~adinn/8224974/webrev.04
>> 
>> n.b. This webrev also includes changes to the javadoc for
>> ExtendedMapMode suggested by Alan Bateman.
> . . .
> 
> I have made one further change to locate the new ExtendedMapMode enum in
> module jdk.nio.mapmode and package jdk.nio.mapmode as suggested offline
> by Alan Bateman and the OpenJDK lead. Please refer to this webrev instead:
> 
> webrev:  http://cr.openjdk.java.net/~adinn/8224974/webrev.05
> 
> The CSR and JEP have been updated accordingly
> 
> CSR:  https://bugs.openjdk.java.net/browse/JDK-8224975
> JEP:  https://bugs.openjdk.java.net/browse/JDK-8207851
> 
> regards,
> 
> 
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190605/b2d196e8/attachment-0001.html>


More information about the nio-dev mailing list