Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-22 Thread Claes Redestad
Lance, Naoto, thanks for reviewing! /Claes On 2020-04-22 18:19, naoto.s...@oracle.com wrote: +1 Naoto On 4/22/20 9:13 AM, Lance Andersen wrote: Hi Claes, The latest version looks good. Thank you for the patch. Best Lance On Apr 22, 2020, at 6:26 AM, Claes Redestad wrote: Hi, new we

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-22 Thread naoto . sato
+1 Naoto On 4/22/20 9:13 AM, Lance Andersen wrote: Hi Claes, The latest version looks good. Thank you for the patch. Best Lance On Apr 22, 2020, at 6:26 AM, Claes Redestad wrote: Hi, new webrev based on discussions here and offline: http://cr.openjdk.java.net/~redestad/8243254/open.01/

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-22 Thread Lance Andersen
Hi Claes, The latest version looks good. Thank you for the patch. Best Lance > On Apr 22, 2020, at 6:26 AM, Claes Redestad wrote: > > Hi, > > new webrev based on discussions here and offline: > > http://cr.openjdk.java.net/~redestad/8243254/open.01/ > > - creates a new testng test TestZipF

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-22 Thread Claes Redestad
Hi, new webrev based on discussions here and offline: http://cr.openjdk.java.net/~redestad/8243254/open.01/ - creates a new testng test TestZipFileEncodings, which derives from TestZipFile rather than repurposing that existing test. There is definitely some overlap, but since TestZipFile is

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-21 Thread Claes Redestad
On 2020-04-21 20:30, Martin Buchholz wrote: > > Are we really willing to maintain tests for UTF-8, ASCII-compatible, and > ASCII-incompatible code paths? If we aren't then I think we should terminally deprecate all the ZipFile constructors that take a Charset argument

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-21 Thread Martin Buchholz
> > > > > > Are we really willing to maintain tests for UTF-8, ASCII-compatible, and > > ASCII-incompatible code paths? > > If we aren't then I think we should terminally deprecate all the ZipFile > constructors that take a Charset argument. Not a call I can make. > I mean more generally. You're

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-21 Thread Claes Redestad
Hi, On 2020-04-21 19:19, Martin Buchholz wrote: Thanks for doing this.  (even though I would not have) at least I learned a few things doing it. :-) I'd be more inclined to have separate code for UTF-8 encoded zip files, that everyone should be using now, rather than have special code for

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-21 Thread Martin Buchholz
Thanks for doing this. (even though I would not have) I'd be more inclined to have separate code for UTF-8 encoded zip files, that everyone should be using now, rather than have special code for ASCII-compatible. Perhaps a single ZipCoder subclass Utf8ZipCoder. I am the author of that test infr

RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

2020-04-21 Thread Claes Redestad
Hi, ZipFile.getEntry does optimizations to check for directory entries by adding a '/' to the encoded byte array. JDK-8242959 improved on this optimization, but a question was raised Jason Zaugg on whether the optimization is always valid[1]. Turns out it isn't, but only for non-ASCII compatible