Quantcast

[10] RFR 8078192: Path2D storage trimming

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
Hi,

Here is a first attempt to propose a Path2D patch (based on JDK10):
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/


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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Prahalad kumar Narayanan
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 <[hidden email]>
To: "[hidden email]" <[hidden email]>, Phil Race
        <[hidden email]>,  Jim Graham <[hidden email]>, Iris
        Clark <[hidden email]>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
        <CAKjRUT7zQ=[hidden email]>
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/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
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 <[hidden email]>:
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 <[hidden email]>
To: "[hidden email]" <[hidden email]>, Phil Race
        <[hidden email]>,  Jim Graham <[hidden email]>, Iris
        Clark <[hidden email]>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
        <CAKjRUT7zQ=[hidden email]>
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/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Prahalad kumar Narayanan
Looks good to me.

Thanks
Have a good day

Prahalad N.

----------------------------------------------------------------------

From: Laurent Bourgès [mailto:[hidden email]]
Sent: Thursday, April 20, 2017 11:51 AM
To: Prahalad Kumar Narayanan
Cc: [hidden email]
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/
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 <[hidden email]>:
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 <[hidden email]>
To: "[hidden email]" <[hidden email]>, Phil Race
        <[hidden email]>,  Jim Graham <[hidden email]>, Iris
        Clark <[hidden email]>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
        <CAKjRUT7zQ=[hidden email]>
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/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
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 <[hidden email]>:
Looks good to me.

Thanks
Have a good day

Prahalad N.

----------------------------------------------------------------------

From: Laurent Bourgès [mailto:[hidden email]]
Sent: Thursday, April 20, 2017 11:51 AM
To: Prahalad Kumar Narayanan
Cc: [hidden email]
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/
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 <[hidden email]>:
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 <[hidden email]>
To: "[hidden email]" <[hidden email]>, Phil Race
        <[hidden email]>,  Jim Graham <[hidden email]>, Iris
        Clark <[hidden email]>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
        <CAKjRUT7zQ=[hidden email]>
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/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Philip Race
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 <[hidden email]>:
Looks good to me.

Thanks
Have a good day

Prahalad N.

----------------------------------------------------------------------

From: Laurent Bourgès [mailto:[hidden email]]
Sent: Thursday, April 20, 2017 11:51 AM
To: Prahalad Kumar Narayanan
Cc: [hidden email]
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/
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 <[hidden email]>:
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 <[hidden email]>
To: "[hidden email]" <[hidden email]>, Phil Race
        <[hidden email]>,  Jim Graham <[hidden email]>, Iris
        Clark <[hidden email]>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
        <CAKjRUT7zQ=[hidden email]>
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/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
Hi Phil,

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.

Ok, no problem. As the CCC was approved for JDK9, I wondered if that was needed again.
 

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.

I agree, it only provides a fluent interface and mimics the clone() approach, but you're right, it is not really needed.
I will remove that and fix the test that uses that method:
        testAddLine(pf.trimToSize(), isEmpty);

Laurent


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 <[hidden email]>:
Looks good to me.

Thanks
Have a good day

Prahalad N.

----------------------------------------------------------------------

From: Laurent Bourgès [mailto:[hidden email]]
Sent: Thursday, April 20, 2017 11:51 AM
To: Prahalad Kumar Narayanan
Cc: [hidden email]
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/
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 <[hidden email]>:
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 <[hidden email]>
To: "[hidden email]" <[hidden email]>, Phil Race
        <[hidden email]>,  Jim Graham <[hidden email]>, Iris
        Clark <[hidden email]>
Subject: [OpenJDK 2D-Dev] [10] RFR 8078192: Path2D storage trimming
Message-ID:
        <CAKjRUT7zQ=[hidden email]>
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/

JBS: 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



--
--
Laurent Bourgès
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Jim Graham-5
In reply to this post by Laurent Bourgès
Hi Laurent,

The implementation looks good, except that the method chaining-style return value seems out of place here.  Similar
trimToSize() methods in Collections return void and none of the other methods in this area use the method chaining
paradigm.  In the interest of maintaining a common design theme throughout 2D this method should just return void.

                        ...jim

On 4/18/17 11:49 PM, Laurent Bourgès wrote:

> Hi,
>
> Here is a first attempt to propose a Path2D patch (based on JDK10):
> http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/
>
> JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
Hi Phil & Jim,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.2/

Changes:
- trimToSize() return void
- fixed test + jtreg passed

Bye,
Laurent

2017-04-20 21:30 GMT+02:00 Jim Graham <[hidden email]>:
Hi Laurent,

The implementation looks good, except that the method chaining-style return value seems out of place here.  Similar trimToSize() methods in Collections return void and none of the other methods in this area use the method chaining paradigm.  In the interest of maintaining a common design theme throughout 2D this method should just return void.

                        ...jim


On 4/18/17 11:49 PM, Laurent Bourgès wrote:
Hi,

Here is a first attempt to propose a Path2D patch (based on JDK10):
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Philip Race
 You have a capital letter here and I think it must be lower case ..


2499      * @Since 10

-phil.

On 4/20/17, 1:58 PM, Laurent Bourgès wrote:
Hi Phil & Jim,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.2/

Changes:
- trimToSize() return void
- fixed test + jtreg passed

Bye,
Laurent

2017-04-20 21:30 GMT+02:00 Jim Graham <[hidden email]>:
Hi Laurent,

The implementation looks good, except that the method chaining-style return value seems out of place here.  Similar trimToSize() methods in Collections return void and none of the other methods in this area use the method chaining paradigm.  In the interest of maintaining a common design theme throughout 2D this method should just return void.

                        ...jim


On 4/18/17 11:49 PM, Laurent Bourgès wrote:
Hi,

Here is a first attempt to propose a Path2D patch (based on JDK10):
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
Sorry for the typo, I added also a newline before @since:


2017-04-20 23:04 GMT+02:00 Philip Race <[hidden email]>:
 You have a capital letter here and I think it must be lower case ..


2499      * @Since 10

-phil.


On 4/20/17, 1:58 PM, Laurent Bourgès wrote:
Hi Phil & Jim,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.2/

Changes:
- trimToSize() return void
- fixed test + jtreg passed

Bye,
Laurent

2017-04-20 21:30 GMT+02:00 Jim Graham <[hidden email]>:
Hi Laurent,

The implementation looks good, except that the method chaining-style return value seems out of place here.  Similar trimToSize() methods in Collections return void and none of the other methods in this area use the method chaining paradigm.  In the interest of maintaining a common design theme throughout 2D this method should just return void.

                        ...jim


On 4/18/17 11:49 PM, Laurent Bourgès wrote:
Hi,

Here is a first attempt to propose a Path2D patch (based on JDK10):
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/

JBS: 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Laurent Bourgès
Sorry (bad shortcut);

Here is the fixed webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.3/

Laurent

2017-04-21 0:04 GMT+02:00 Laurent Bourgès <[hidden email]>:
Sorry for the typo, I added also a newline before @since:


2017-04-20 23:04 GMT+02:00 Philip Race <[hidden email]>:
 You have a capital letter here and I think it must be lower case ..


2499      * @Since 10

-phil.


On 4/20/17, 1:58 PM, Laurent Bourgès wrote:
Hi Phil & Jim,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.2/

Changes:
- trimToSize() return void
- fixed test + jtreg passed

Bye,
Laurent

2017-04-20 21:30 GMT+02:00 Jim Graham <[hidden email]>:
Hi Laurent,

The implementation looks good, except that the method chaining-style return value seems out of place here.  Similar trimToSize() methods in Collections return void and none of the other methods in this area use the method chaining paradigm.  In the interest of maintaining a common design theme throughout 2D this method should just return void.

                        ...jim


On 4/18/17 11:49 PM, Laurent Bourgès wrote:
Hi,

Here is a first attempt to propose a Path2D patch (based on JDK10):
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/

JBS: 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



--
--
Laurent Bourgès
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR 8078192: Path2D storage trimming

Philip Race
OK. Although we still need to wait for the CSR process.

-phil.

On 4/20/17, 3:05 PM, Laurent Bourgès wrote:
Sorry (bad shortcut);

Here is the fixed webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.3/

Laurent

2017-04-21 0:04 GMT+02:00 Laurent Bourgès <[hidden email]>:
Sorry for the typo, I added also a newline before @since:


2017-04-20 23:04 GMT+02:00 Philip Race <[hidden email]>:
 You have a capital letter here and I think it must be lower case ..


2499      * @Since 10

-phil.


On 4/20/17, 1:58 PM, Laurent Bourgès wrote:
Hi Phil & Jim,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.2/

Changes:
- trimToSize() return void
- fixed test + jtreg passed

Bye,
Laurent

2017-04-20 21:30 GMT+02:00 Jim Graham <[hidden email]>:
Hi Laurent,

The implementation looks good, except that the method chaining-style return value seems out of place here.  Similar trimToSize() methods in Collections return void and none of the other methods in this area use the method chaining paradigm.  In the interest of maintaining a common design theme throughout 2D this method should just return void.

                        ...jim


On 4/18/17 11:49 PM, Laurent Bourgès wrote:
Hi,

Here is a first attempt to propose a Path2D patch (based on JDK10):
http://cr.openjdk.java.net/~lbourges/path2D/Path2D-8078192.0/

JBS: 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



--
--
Laurent Bourgès
Loading...