Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-15 Thread Claes Redestad
On 03/15/2017 07:33 PM, Mandy Chung wrote: On Mar 15, 2017, at 11:25 AM, Claes Redestad wrote: On 03/15/2017 05:36 PM, Mandy Chung wrote: I would prefer to separate the performance issue from this fix and keep this fix simple. There may be more to tease out for performance regression tha

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-15 Thread Mandy Chung
> On Mar 15, 2017, at 11:25 AM, Claes Redestad > wrote: > > > > On 03/15/2017 05:36 PM, Mandy Chung wrote: >> I would prefer to separate the performance issue from this fix and keep this >> fix simple. There may be more to tease out for performance regression that >> I defer to Sherman to

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-15 Thread Xueming Shen
The import j.u.Objects is no longer needed? The rest looks fine. -Sherman On 3/15/17, 11:25 AM, Claes Redestad wrote: On 03/15/2017 05:36 PM, Mandy Chung wrote: I would prefer to separate the performance issue from this fix and keep this fix simple. There may be more to tease out for perf

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-15 Thread Claes Redestad
On 03/15/2017 05:36 PM, Mandy Chung wrote: I would prefer to separate the performance issue from this fix and keep this fix simple. There may be more to tease out for performance regression that I defer to Sherman to discuss with you. Ok: http://cr.openjdk.java.net/~redestad/8176709/jdk.0

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-15 Thread Mandy Chung
> On Mar 15, 2017, at 4:06 AM, Claes Redestad wrote: > > Hi Mandy, > > On 03/15/2017 12:32 AM, Mandy Chung wrote: >> I agree with the goal to reduce the number of qualified exports, which I >> always like to keep. >> >> Duplicating code like this isn’t ideal although it’s straight-forward. T

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-15 Thread Claes Redestad
Hi Mandy, On 03/15/2017 12:32 AM, Mandy Chung wrote: I agree with the goal to reduce the number of qualified exports, which I always like to keep. Duplicating code like this isn’t ideal although it’s straight-forward. This is a performance optimization. One solution is to keep using the Mani

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-14 Thread Mandy Chung
I agree with the goal to reduce the number of qualified exports, which I always like to keep. Duplicating code like this isn’t ideal although it’s straight-forward. This is a performance optimization. One solution is to keep using the Manifest API and check the attribute value equals to `true

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-14 Thread Claes Redestad
Hi, Alan raised some concerns offline that we should try to reduce the number of qualified exports, not adding more, and that it might be better to accept some code duplication here. Thus I'm proposing this as an alternative: http://cr.openjdk.java.net/~redestad/8176709/jdk.02/ Neither solution

RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

2017-03-14 Thread Claes Redestad
Hi, please review this change to adapt the JarFileSystem::isMultiReleaseJar method to align with the evolved specification in JEP 238 Bug: https://bugs.openjdk.java.net/browse/JDK-8176709 Webrev: http://cr.openjdk.java.net/~redestad/8176709/jdk.01/ This unfortunately adds a qualified export fro