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.naraya...@oracle.com
<mailto:prahalad.kumar.naraya...@oracle.com>>:
Looks good to me.
Thanks
Have a good day
Prahalad N.
----------------------------------------------------------------------
From: Laurent Bourgès [mailto:bourges.laur...@gmail.com
<mailto:bourges.laur...@gmail.com>]
Sent: Thursday, April 20, 2017 11:51 AM
To: Prahalad Kumar Narayanan
Cc: 2d-dev@openjdk.java.net <mailto:2d-dev@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.naraya...@oracle.com
<mailto:prahalad.kumar.naraya...@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.laur...@gmail.com
<mailto:bourges.laur...@gmail.com>>
To: "2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>"
<2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>>, Phil Race
<philip.r...@oracle.com <mailto:philip.r...@oracle.com>>, Jim
Graham <james.gra...@oracle.com <mailto:james.gra...@oracle.com>>,
Iris
Clark <iris.cl...@oracle.com <mailto:iris.cl...@oracle.com>>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
<CAKjRUT7zQ=zsqiqdbpdtoecosfolvpzqhdvi-pxzsmwtgkr...@mail.gmail.com
<mailto:zsqiqdbpdtoecosfolvpzqhdvi-pxzsmwtgkr...@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