RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect
mandy.chung at oracle.com
Wed Mar 15 16:36:00 UTC 2017
> On Mar 15, 2017, at 4:06 AM, Claes Redestad <claes.redestad at oracle.com> 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. This is a performance optimization. One solution is to keep using the Manifest API and check the attribute value equals to `true` and separate the performance issue and explore any other solution. Perhaps parsing of Manifest could be optimized.
> yes, this enhances the performance of JarFileSystem::isMultiReleaseJar, which isn't strictly necessary
> for the bug fix at hand, but based on that fact that the code in JarFile has been thoroughly tested
> I figured this to be the lowest risk change rather than figuring out how to make the current
> Manifest-reading code correct.
> At a glance then making the existing code correct is likely trivial and minimal as you say, but I was
> simply more comfortable copying/reusing code that I know works than delving into the details of an
> implementation with which I'm less familiar.
> If you insist and can review promptly I'll go ahead and make a minimal fix tonight, but I'd prefer to go
> ahead with this well-known code before we hit RDP2 tomorrow.
>  While technically having JarFileSystem read the Manifest on creation *is* a performance regression
> since that's never done in 8, I have no proof that this is in any way a critical regression.
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.
More information about the core-libs-dev