[OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Philip Race
philip.race at oracle.com
Thu Apr 20 17:13:17 UTC 2017
Hi,
This can't be pushed without a CSR approval for JDK10 and that process won't
be "up and running" for about another 10 days.
But before we get to that I wonder why we need to return Path2D when we
aren't creating a new one .. there doesn't seem to be any value to it and
it just potentially misleading a caller into thinking it has the cost of
doing so.
If any value is worth returning it might be a boolean to indicate
whether any trimming
actually occurred.
-phil.
On 4/20/17, 4:41 AM, Laurent Bourgès wrote:
> Phil,
>
> Do you approve the patch and the javadoc too ?
>
> If yes, could you push it for me into OpenJDK10 ?
>
> Laurent
>
>
> 2017-04-20 11:55 GMT+02:00 Prahalad Kumar Narayanan
> <prahalad.kumar.narayanan at oracle.com
> <mailto:prahalad.kumar.narayanan at oracle.com>>:
>
> Looks good to me.
>
> Thanks
> Have a good day
>
> Prahalad N.
>
> ----------------------------------------------------------------------
>
> From: Laurent Bourgès [mailto:bourges.laurent at gmail.com
> <mailto:bourges.laurent at gmail.com>]
> Sent: Thursday, April 20, 2017 11:51 AM
> To: Prahalad Kumar Narayanan
> Cc: 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage
> trimming
>
> Hello,
> Here is an updated webrev:
> http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.1/
> <http://cr.openjdk.java.net/%7Elbourges/path2D/Path2D-8078192.1/>
> I fixed the copyright dates and the javadoc as you proposed (I
> initially derived the javadoc from Vector.trimToSize)
> Thanks,
> Laurent
>
> 2017-04-20 5:46 GMT+02:00 Prahalad Kumar Narayanan
> <prahalad.kumar.narayanan at oracle.com
> <mailto:prahalad.kumar.narayanan at oracle.com>>:
> Hello Laurent
>
> The code changes look good.
> . The new method trimToSize effectively does the same
> operation (Arrays.copyOf) as done by the copy contructors.
> . The test file- Path2DCopyConstructor now tests after
> executing the methods pathObj.trimToSize and pathObj.clone
>
> With regard to the text in Javadoc comments
> . I felt, the first line clearly explains the operation -
> "Trims the capacity of this Path2D instance to its
> current size."
> . The immediate following line is redundant and may not be
> required -
> "If the capacity .. is larger than its current
> size... "
> . The third line will be required till the end of the comments -
> "An application can use ... minimize the storage of
> a path. ... since 10"
> . This is my observation. You could wait for other suggestions.
>
> Btw, the copyright year should be changed to 2017 in both
> Path2D.java and the test file.
> (Some use automation scripts that modify the year at the time of
> check-in. If so, kindly ignore the observation)
>
> Thanks
> Have a good day
>
> Prahalad N.
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Wed, 19 Apr 2017 08:49:33 +0200
> From: Laurent Bourg?s <bourges.laurent at gmail.com
> <mailto:bourges.laurent at gmail.com>>
> To: "2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>"
> <2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>>, Phil Race
> <philip.race at oracle.com <mailto:philip.race at oracle.com>>, Jim
> Graham <james.graham at oracle.com <mailto:james.graham at oracle.com>>,
> Iris
> Clark <iris.clark at oracle.com <mailto:iris.clark at oracle.com>>
> Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
> Message-ID:
> <CAKjRUT7zQ=ZSqiQdbpdtOeCOSFoLVPzqHdvi-pXzsmwTgKRbDw at mail.gmail.com <mailto:ZSqiQdbpdtOeCOSFoLVPzqHdvi-pXzsmwTgKRbDw at mail.gmail.com>>
> Content-Type: text/plain; charset="utf-8"
>
> Hi,
>
> Here is a first attempt to propose a Path2D patch (based on JDK10):
> http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/
> <http://cr.openjdk.java.net/%7Elbourges/path2D/Path2D-8078192.0/>
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8078192
> <https://bugs.openjdk.java.net/browse/JDK-8078192>
>
> Please review the Path2D changes, notably the javadoc (english)
> and the modified Path2DCopyConstructor test which checks all
> public Path2D methods on concrete classes (Path2D.Float,
> Path2D.Double, GeneralPath) after calling path.trimToSize()
>
> Cheers,
> Laurent
>
>
>
> --
> --
> Laurent Bourgès
>
>
>
>
> --
> --
> Laurent Bourgès
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170420/eb41e0be/attachment.html>
More information about the 2d-dev
mailing list