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
<[email protected]
<mailto:[email protected]>>:
Looks good to me.
Thanks
Have a good day
Prahalad N.
----------------------------------------------------------------------
From: Laurent Bourgès [mailto:[email protected]
<mailto:[email protected]>]
Sent: Thursday, April 20, 2017 11:51 AM
To: Prahalad Kumar Narayanan
Cc: [email protected] <mailto:[email protected]>
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
<[email protected]
<mailto:[email protected]>>:
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 <[email protected]
<mailto:[email protected]>>
To: "[email protected] <mailto:[email protected]>"
<[email protected] <mailto:[email protected]>>, Phil Race
<[email protected] <mailto:[email protected]>>, Jim
Graham <[email protected] <mailto:[email protected]>>,
Iris
Clark <[email protected] <mailto:[email protected]>>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
<CAKjRUT7zQ=zsqiqdbpdtoecosfolvpzqhdvi-pxzsmwtgkr...@mail.gmail.com
<mailto:[email protected]>>
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
