Re: RFR: 8242842: Avoid reallocating name when checking for trailing slash in ZipFile.getEntryPos
Hi Claes, I think this looks good overall. Vwar LNXW > On Apr 15, 2020, at 8:58 AM, Claes Redestad wrote: > > Hi, > > a trivial optimization to ZipFile.getEntryPos extracted from some > experiments conducted mostly by Eirik Bjørsnøs. > > Webrev: http://cr.openjdk.java.net/~redestad/8242842/open.00/ > Bug:https://bugs.openjdk.java.net/browse/JDK-8242842 > > This removes an extra array copy on every miss, and the patch means a > small speed-up on a few startup applications I've tested (Spring > Petclinic: ~50ms faster). > > This is an optimization which was lost in translation when porting from > native to Java[1] in JDK 9. The native implementation allocates an > array that can fit the extra '/' if needed, and updates the array in > place for the follow-up check. > > Thanks! > > /Claes > > [1] https://bugs.openjdk.java.net/browse/JDK-8145260 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8242596: Improve JarFile.getEntry performance for multi-release jar
Hi Claes, Hi Claes, I think this looks good. I few minor comments/suggestions: JavaUtilZipFileAccess.java:Please update the copyright JarFile.java: In getVersionEntry(), can we place break the if statements onto two lines to make it a bit easier to read? ZipFile.java: Perhaps add a comment around 690 as to why this code was added for future maintainers Best Lance > On Apr 16, 2020, at 8:48 AM, Claes Redestad wrote: > > Hi, > > please review this patch to improve JarFile.getEntry performance > on multi-release jar files, mainly contributed by Eirik Bjørsnøs. > > The main idea is to piggy-back on the scanning of meta-inf entries > we already do during initCEN to scan what versions exists in the > jar file. This then reduces the number of lookups we'll have to do > in typical scenarios. > > Webrev: http://cr.openjdk.java.net/~redestad/8242596/open.00/ > Bug:https://bugs.openjdk.java.net/browse/JDK-8242596 > > Testing: tier1-2 > > Thanks! > > /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8242596: Improve JarFile.getEntry performance for multi-release jar
Looks good thank you Claes > On Apr 16, 2020, at 11:27 AM, Claes Redestad > wrote: > > Lance, > > On 2020-04-16 16:33, Lance Andersen wrote: >> I think this looks good. I few minor comments/suggestions: >> * JavaUtilZipFileAccess.java:Please update the copyright >> * JarFile.java: In getVersionEntry(), can we place break the if >>statements onto two lines to make it a bit easier to rea > * >> ZipFile.java: Perhaps add a comment around *1490* as to why this > code >>was added for future maintainers > > done: http://cr.openjdk.java.net/~redestad/8242596/open.01 > > Thanks! > > /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8242959: Optimize ZipFile.getEntry by folding lookups for name and name+'/'
HI Claes, I think this looks good. Best Lance > On Apr 17, 2020, at 7:33 AM, Claes Redestad wrote: > > Hi, > > please review this patch to get rid of the back-to-back lookup of name > and name+'/' in ZipFile.getEntry. This is done by first adjusting the > hash function so that a trailing slash is not included in an entry's > hash code, and adjusting appropriately when matching the arrays. > > This means we do fewer table lookups, less arithmetic and potentially > less allocation on lookup misses. > > Patch contributed by Eirik Bjørsnøs. > > Webrev: http://cr.openjdk.java.net/~redestad/8242959/open.00/ > Bug:https://bugs.openjdk.java.net/browse/JDK-8242959 > > Testing: tier1+2, verified neutral performance for hits and a > significant reduction in cost of misses (~30-50ms improvement on > Spring PetClinic startup). > > Thanks! > > /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 15: 8243010: Test support: Customizable Hex Printer
Hi Roger, Thank you for creating this as it looks good. A couple of quick questions - Will the javadoc be hosted somewhere - Maybe I have not stumbled onto it yet, but is there a document/wiki somewhere that describes all of the useful test libraries vs discovering as you go? Best Lance > On Apr 16, 2020, at 3:17 PM, Roger Riggs wrote: > > Please review[2] and comment on a new Hex printing utility to support OpenJDK > tests. > The usefulness of a hex printing has been discussed from time to time with > many suggestions as to the API shape and features. > > To get an idea of the API and features, take a look at the javadoc[1]. > It covers the basic formatting of offset, and values, and extends the > descriptive part beyond simple ASCII or printable so that a custom formatter > can interpret the byte stream in parallel to the bytes. > > The webrev includes changes to existing tests that currently > use HexDumpEncoder to use HexPrinter. > > Comments appreciated, Roger > > [1]Javadoc: > http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/javadoc > > [2] Webrev: > http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/ > > [3] Issue: > https://bugs.openjdk.java.net/browse/JDK-8243010 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 15: 8243010: Test support: Customizable Hex Printer
Hi Roger, I think tackling this via the new Developer’s guide project makes sense as it would help folks re-inventing the wheel and making it easier to find what already exists. Best Lance > On Apr 17, 2020, at 12:55 PM, Roger Riggs wrote: > > Hi Lance, > > The test library is a bit of a black box usually approached from the angle of > existing tests. > And unfortunately, there is a lower bar for documentation and API development. > Perhaps an item to be looked at as part of the new developer guide project. > > Thanks, Roger > > > > On 4/17/20 11:34 AM, Lance Andersen wrote: >> Hi Roger, >> >> Thank you for creating this as it looks good. >> >> A couple of quick questions >> >> - Will the javadoc be hosted somewhere >> - Maybe I have not stumbled onto it yet, but is there a document/wiki >> somewhere that describes all of the useful test libraries vs discovering as >> you go? >> >> >> Best >> Lance >> >> >>> On Apr 16, 2020, at 3:17 PM, Roger Riggs >> <mailto:roger.ri...@oracle.com>> wrote: >>> >>> Please review[2] and comment on a new Hex printing utility to support >>> OpenJDK tests. >>> The usefulness of a hex printing has been discussed from time to time with >>> many suggestions as to the API shape and features. >>> >>> To get an idea of the API and features, take a look at the javadoc[1]. >>> It covers the basic formatting of offset, and values, and extends the >>> descriptive part beyond simple ASCII or printable so that a custom formatter >>> can interpret the byte stream in parallel to the bytes. >>> >>> The webrev includes changes to existing tests that currently >>> use HexDumpEncoder to use HexPrinter. >>> >>> Comments appreciated, Roger >>> >>> [1]Javadoc: >>> http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/javadoc >>> <http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/javadoc> >>> >>> [2] Webrev: >>> http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/ >>> <http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/> >>> >>> [3] Issue: >>> https://bugs.openjdk.java.net/browse/JDK-8243010 >>> <https://bugs.openjdk.java.net/browse/JDK-8243010> >> >> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| >> Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> >> >> >> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()
JBS issue so we can > continue the discussion there. > >> found an instance of 8096, which is either a typo or a clever >> optimization to keep the array + array object header fit snugly within >> 8Kb. You chose. :-) >> > > I like how you try to be positive :) > >>> >>> Thank you and best regards, >>> Volker >>> >>> PS: do you think it makes sense to contribute the benchmark attached >>> to JDK-8242864 to the code-tools/mh-jdk-microbenchmarks [1] project? >>> >>> [1]http://openjdk.java.net/projects/code-tools/jmh-jdk-microbenchmarks/ >>> <http://openjdk.java.net/projects/code-tools/jmh-jdk-microbenchmarks/> >> >> I'd definitely welcome the micro as part of the patch under >> test/micro/org/openjdk/bench/java/util/zip - additionally contributing > > I knew that "jmh-jdk-microbenchmarks" has been copied to the jdk repo > but somehow I did found "jmh-jdk-microbenchmarks" before looking in > the obvious place :) > > So I've added the benchmark to the patch now. There's one thing I'm > not sure however. The benchmark requires a "big" (several 100k) with > good compression ratio (e.g. a large text file). I've decided to use a > big Java source file from "src.zip" but I'm not sure if "src.zip" is > always available in the jdk images which are used to run the > microbenchmarks. Do you think the test it is fine this way or do you > have a better idea? I saw that "ZipFind" uses "microbenchmarks.jar" > (i.e. the container of the test itself) but that file is already > compressed so the compression rate won't be that good. > > Another thing I couldn't figure out is a good way to skip the > benchmark when I realize that I can't load the expected file in the > "@Setup" method. Do you now anything better than just throwing an > exception? > > Thank you and best regards, > Volker > >> to jmh-jdk-microbenchmarks could enable you to test the micro on 8 or >> 11. >> >> /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets
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 TestZipFileEncodings, which derives from > TestZipFile rather than repurposing that existing test. There is > definitely some overlap, but since TestZipFile is only run manually > and this new test runs fine (and quickly) in our testing I think > that's fine > > - only implements the optimization for UTF-8, removing the concept > of ASCII-compatible ZipCoders. > > - a few other cleanups > > Testing: tier1+2 > > Thanks! > > /Claes > > On 2020-04-21 15:52, Claes Redestad wrote: >> 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 charsets. >> While JarFiles are always assumed to be UTF-8-encoded or compatible, >> ZipFiles allow arbitrary encoding. E.g., UTF-16 would encode '/' (2F) as >> either 2F 00 or 00 2F, which means the hash code would differ and a >> directory "foo/" potentially not be found when looking up "foo". Further >> complications arise when/if the directory name ends with a code point >> that might be encoded so that the final byte is 2F, e.g. \u012F. >> This patch only enables this low-level optimization when the charset >> encoding used is known to be ASCII compatible in the sense that 2F will >> be encoded as single-byte 2F. UTF-8 is compatible in this sense - and >> since this is the charset exclusively used by JarFile these changes have >> little to no effect on startup performance in the cases we've been >> looking at. >> Webrev: http://cr.openjdk.java.net/~redestad/8243254/open.00/ >> Bug:https://bugs.openjdk.java.net/browse/JDK-8243254 >> I've partially repurposed ZipFile/TestZipFile to test some such corner >> cases. Mainly expanded it to test ZipFiles created with various non- >> standard encodings, but also slimmed it down so that it can actually run >> quickly and reliably - as well as enabled it in regular testing. The >> updated test fails both before and after JDK-8242959, but passes with >> the proposed changes. >> Testing: tier1+2 >> Thanks! >> /Claes >> [1] https://twitter.com/retronym/status/1252134723431747584?s=20 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()
Hi Volker
> On Apr 22, 2020, at 4:08 PM, Volker Simonis wrote:
>
> On Tue, Apr 21, 2020 at 5:23 PM Lance Andersen
> wrote:
>>
>> Hi Volker,
>>
>> I think overall this looks OK. I went through the older SCCS histories to
>> see if I could figure out why they were using 512 for the input length but
>> could not find anything that might shed some light for me.
>>
>
> Hi Lance,
>
> thanks a lot for digging in the old sources to review my change. It's
> great that we stil have people who can use SCCS :)
:-)
>
>> I am not sure you can guarantee that src.zip exists but others might be able
>> to weigh in here. What we have been trying to do going forward is to have
>> the tests create the zip files that it needs. In some cases, we have
>> created a byte array within the test which represents the zip and just write
>> it out before the test begins.
>>
>
> Yes, the dependency on an external file was not nice, so I rewrote the
> benchmark to programmatically create a file which can be compressed by
> a factor of ~6:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.02/
>
> Notice that this new version only changes the microbenchmark, all the
> other files are untouched.
>
> As everybody seemed to be happy with the change itself and the
> regression test, I'm now waiting for your and Clae's final review of
> the microbenchmark before pushing. Please let me know hat you think?
I think you are good to go from my POV.
Thank you for your contribution to improving Zip performance.
Best,
Lance
>
> Best regards,
> Volker
>
>> I am hoping others with more history might also chime in case they are aware
>> of the history here.
>>
>> Thank you for helping improve the performance.
>>
>> Best
>> Lance
>>
>> On Apr 17, 2020, at 6:49 AM, Volker Simonis wrote:
>>
>> Thanks everybody for looking at this change!
>>
>> Please find an updated webrev (with a new test and micro-benchmark)
>> and my answers to your comments below:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.01/
>>
>> On Thu, Apr 16, 2020 at 6:40 AM Vyom Tiwari wrote:
>>
>> Thanks for doing this, i think the below code change is not required .
>>
>> Please do let me know if i am not reading it correctly.
>>
>> if (inf.finished() || (len == 0)/* no more input */) {
>>
>> If you check the native code "inf.finished() will be true only of the
>> corresponding native call inflate(strm, Z_PARTIAL_FLUSH) returns
>> "Z_STREAM_END".
>>
>> After your code change write may return even if finished() is not true.
>>
>>
>> Yes, that's true, but that's what we must do if there's no more input
>> available. Before my change this break on "len < 1" was in the "if
>> (inf.needsInput())" branch.
>>
>> On Thu, Apr 16, 2020 at 8:22 AM Thomas Stüfe wrote:
>>
>> 252 // Check the decompressor
>> 253 if (inf.finished() || (len == 0)/* no more input */) {
>> 254 break;
>> 255 }
>>
>> Not sure but I think this is wrong because now you bypass the followup
>> handling of inf.needsDirectory.
>>
>> Inflater.inflate() returns 0 if either needsInput or needsDirectory. We have
>> to ignore needsInput since we have no input anymore, but needsDirectory has
>> to be handled, no?
>>
>>
>> You're absolutely right Thomas. Thanks for catching this! I've moved
>> the check for "needsDictionary" in front of the "finished() || len ==
>> 0" check.
>>
>> Unfortunately there is not very good test coverage for zip with preset
>> dictionaries (jtreg and submit repo passed without problems). So I
>> added a new test for this use case to "
>> test/jdk/java/util/zip/DeflateIn_InflateOut.java".
>>
>> On Thu, Apr 16, 2020 at 8:48 AM Thomas Stüfe wrote:
>>
>>
>> As for increasing the buffer size, it makes sense but IMHO needs a CSR and a
>> release note.
>>
>>
>> I don't think so. This is an internal, implementation specific setting
>> which has never been exposed or documented before so I don't see why
>> we should document it now. But let's discuss this separately in the
>> corresponding JBS issue (see below).
>>
>> On Thu, Apr 16, 2020 at 1:18 PM Claes Redestad
>> wrote:
>>
>>
>> Hi Volker,
>>
>> On 2020-04-15 19
Re: RFR 15: 8243010: Test support: Customizable Hex Printer
Hi Roger, Sorry, I thought I replied. I think this looks good and I look forward to using it more going forward. +1 from me Best Lance > On Apr 23, 2020, at 2:46 PM, Roger Riggs wrote: > > Ping... > > I appreciate the comments about extending the functionality and uses and will > take them up as a separate enhancement. > > A review would be appreciated. > > Thanks, Roger > > > On 4/16/20 3:17 PM, Roger Riggs wrote: >> Please review[2] and comment on a new Hex printing utility to support >> OpenJDK tests. >> The usefulness of a hex printing has been discussed from time to time with >> many suggestions as to the API shape and features. >> >> To get an idea of the API and features, take a look at the javadoc[1]. >> It covers the basic formatting of offset, and values, and extends the >> descriptive part beyond simple ASCII or printable so that a custom formatter >> can interpret the byte stream in parallel to the bytes. >> >> The webrev includes changes to existing tests that currently >> use HexDumpEncoder to use HexPrinter. >> >> Comments appreciated, Roger >> >> [1]Javadoc: >> http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/javadoc >> >> [2] Webrev: >> http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/ >> >> [3] Issue: >> https://bugs.openjdk.java.net/browse/JDK-8243010 > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8243469: Lazily encode name in ZipFile.getEntryPos
Hi Claes, Thank you for continuing to improve the performance in this part of java.util.zip. The changes I think look pretty good. Best Lance > On Apr 23, 2020, at 8:34 AM, Claes Redestad wrote: > > Hi, > > current implementation of ZipFile.getEntryPos takes the encoded byte[] > of the String we're looking up. Which means when looking up entries > across multiple jar files, we allocate the byte[] over and over for each > jar file searched. > > If we instead refactor the internal hash table to use a normalized > String-based hash value, we can almost always avoid both the repeated > encoding and (most of) the hash calculation when the entry is not found > in the jar/zip. > > This realizes a significant startup improvement on applications with > several or many jar files. A rather typical case. For example I see a > 25% speedup of ZipEnty.getEntry calls on a Spring PetClinic, which > reduces total startup time by ~120ms, or ~2.5% of total, on my setup. At > the same time remaining neutral on single-jar apps. > > Webrev: http://cr.openjdk.java.net/~redestad/8243469/open.00/ > Bug:https://bugs.openjdk.java.net/browse/JDK-8243469 > > Testing: tier1-2 > > Thanks! > > /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8243469: Lazily encode name in ZipFile.getEntryPos
char[] decoded = cb.array();
>for (int i = 0; i < limit; i++) {
>h = 31 * h + decoded[i];
>}
>if (limit > 0 && decoded[limit - 1] != '/') {
>h = 31 * h + '/';
>}
>
> An assert seems like overkill.
>
>> Zipcoder.get() seems to be the only remaining if block without braces.
>> Maybe you'll wnat to fix that once your on it?
>> public static ZipCoder get(Charset charset) {
>> if (charset == UTF_8.INSTANCE)
>> return UTF8;
>> return new ZipCoder(charset);
>
> Sure.
>
> /Claes
>
>> Thumbs up from my side. There's no need for a new webrev from my side.
>> Best regards,
>> Volker
>>> Testing: tier1-4
>>>
>>> Thanks!
>>>
>>> /Claes
>>>
>>> [1]
>>> Baseline:
>>> Benchmark (size) Mode Cnt Score Error Units
>>> ZipFileGetEntry.getEntryHit 512 avgt 15 126.264 ± 5.297
>>> ns/op
>>> ZipFileGetEntry.getEntryHit 1024 avgt 15 130.823 ± 7.212
>>> ns/op
>>> ZipFileGetEntry.getEntryHitUncached 512 avgt 15 152.149 ± 4.978
>>> ns/op
>>> ZipFileGetEntry.getEntryHitUncached 1024 avgt 15 151.527 ± 4.054
>>> ns/op
>>>
>>> open.01:
>>> Benchmark (size) Mode CntScore Error
>>> Units
>>> ZipFileGetEntry.getEntryHit 512 avgt 15 84.450 ± 5.474
>>> ns/op
>>> ZipFileGetEntry.getEntryHit 1024 avgt 15 85.224 ± 3.776
>>> ns/op
>>> ZipFileGetEntry.getEntryHitUncached 512 avgt 15 140.448 ± 4.667
>>> ns/op
>>> ZipFileGetEntry.getEntryHitUncached 1024 avgt 15 145.046 ± 7.363
>>>
>>> [2] I stopped short of taking the cleanup a step further by decoding to
>>> String even in initCEN, which sadly isn't performance neutral:
>>>
>>> http://cr.openjdk.java.net/~redestad/8243469/open.01.init_decode/
>>>
>>> Something for the future to consider, maybe.
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: [XXS] JDK-8243598: Typos in java.lang.invoke package-info
+1
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Sent from my iPhone
> On Apr 28, 2020, at 8:53 PM, Mandy Chung wrote:
>
> diff --git a/src/java.base/share/classes/java/lang/invoke/package-info.java
> b/src/java.base/share/classes/java/lang/invoke/package-info.java
> --- a/src/java.base/share/classes/java/lang/invoke/package-info.java
> +++ b/src/java.base/share/classes/java/lang/invoke/package-info.java
> @@ -31,7 +31,7 @@
> * As described in the Java Virtual Machine Specification, certain types in
> this package
> * are given special treatment by the virtual machine:
> *
> - * The classes {@link java.lang.invoke.MethodHandle MethodHandle}
> + * The classes {@link java.lang.invoke.MethodHandle MethodHandle} and
> * {@link java.lang.invoke.VarHandle VarHandle} contain
> * signature polymorphic methods
> * which can be linked regardless of their type descriptor.
> @@ -190,7 +190,7 @@
> * invoked with just the parameter types of static arguments, thereby
> supporting a wider
> * range of methods compatible with the static arguments (such as methods
> that don't declare
> * or require the lookup, name, and type meta-data parameters).
> - * For example, for dynamically-computed call site, a the first argument
> + * For example, for dynamically-computed call site, the first argument
> * could be {@code Object} instead of {@code MethodHandles.Lookup}, and the
> return type
> * could also be {@code Object} instead of {@code CallSite}.
> * (Note that the types and number of the stacked arguments limit
>
>
> Mandy
Re: RFR: 8244624: Improve handling of JarFile META-INF resources
Hi Claes, I think this looks good. One suggestion before you finalize and push, perhaps update the comment in ZipFile // Use the "oldest ASCII trick in the book" — to include something that this is for lowercase comparison. It just might help others when they look at the code and do not know the trick ;-) What made me think about this was the addition of isManifestName() which also uses this means to lowercase. Best Lance > On May 8, 2020, at 6:57 AM, Claes Redestad wrote: > > Hi Max, > > On 2020-05-08 09:08, Weijun Wang wrote: >> JarFile.java: >> 734: extra new line? > > Fixed > >> 930: Remove or rewrite the comment. > > Did even better: now that I find the position of the manifest during > initCEN, this method should always call JUZFA.getManifest(this, false); > - which is both a simplification and an optimization. > >> ZipFile.java: >> 49: seems not used > > Fixed > >> Please add links to each other between >> src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java::isBlockOrSF >> and >> src/java.base/share/classes/java/util/zip/ZipFile.java::isSignatureRelated. >> The 2nd method can also be static. >> I assume you cannot put ZipFile::isSignatureRelated into >> SignatureFileVerifier.java, right? > > Right, that wouldn't be great for either since ZipFile operates directly > on UTF-8 encoded bytes for performance, while SignatureFileVerifier > works on Strings. > > What we can do though is to add in an assert that the result of > ZF::isSignatureRelated matches that of SFV::isBlockOrSF - which should > ensure. I also added a note to SignatureFileVerifier::isBlockOfSF to > keep these in sync: > > http://cr.openjdk.java.net/~redestad/8244624/open.01/ > > Testing: re-running tier1+2 > >> Thanks, >> Max >> p.s. How do you run the benchmark test? Both locally and on mach5. > > See doc/testing.md > > Basically: > Configure with --with-jmh (jib does this automatically) > make build-microbenchmark > $JDK/bin/java -jar build/$CONF/images/test/micro/benchmarks.jar JarFileMeta > > Thanks! > > /Claes >>> On May 8, 2020, at 5:28 AM, Claes Redestad >>> wrote: >>> >>> Hi, >>> >>> currently during ZipFile creation, we create an int[] array of pointers >>> to META-INF entries. These are then retrieved from three different >>> places in JarFile. >>> >>> However, JarFile is only interested in some combination a few things: >>> the existence of and name of the META-INF/MANIFEST file, the existence >>> of and the names of various signature related files, i.e., files in >>> META-INF that have a suffix such as .EC, .SF, .RSA and .DSA >>> >>> Refactoring the contract between JarFile and ZipFile means we can filter >>> out such entries that we're not interested when opening the file, and >>> also remove the need to create the String for each entries unless we >>> actually want them: >>> >>> Bug:https://bugs.openjdk.java.net/browse/JDK-8244624 >>> Webrev: http://cr.openjdk.java.net/~redestad/8244624/open.00/ >>> >>> This reduces retained footprint of Jar-/ZipFile by slimming down or >>> removing the Source.metanames array entirely, and a significant speed-up >>> in some cases. >>> >>> In the provided microbenchmark, getManifestFromJarWithManifest and >>> getManifestFromJarWithSignatureFiles doesn't call >>> JUZFA.getMetaInfEntryNames but still see small (~5%) speed-up decrease >>> in allocations. >>> >>> getManifestFromJarWithNoManifest exercise JUZFA.getMetaInfEntryNames in >>> the baseline, and now calls JUZFA.getManifestName. Result is a 1.25x >>> speed-up and 30% reduction in allocations. While unrealistic (most JARs >>> have a META-INF/MANIFEST.MF), this speed-up will translate to a few >>> places - such as when loading classes from potentially-signed JAR files. >>> >>> Testing: tier1-2 >>> >>> Thanks! >>> >>> /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi Vipin,
When you submit your revised patch addressing the input previously provided,
please include a test case which exercises the various methods.
Best
Lance
> On May 11, 2020, at 7:54 AM, Alan Bateman wrote:
>
> On 11/05/2020 12:44, Claes Redestad wrote:
>> Hi Vipin,
>>
>> making containsKey("key") return true without also ensuring the
>> other Map operations like get, put, .. work consistently and
>> transparently with String keys seem like a partial fix that will subtly
>> break operations like getOrDefault.
> Yeah, I think this one will require going through Attributes all the
> consistency issues. The outcome may require a spec clarification to several
> methods where it's not currently clear if the input is a String or a Name.
>
> -Alan
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8193066: Avoid use of capturing lambdas in JarFile
Hi Claes, I think this looks good. Thank you for making JarFile zippier ;-) > On May 11, 2020, at 5:50 AM, Claes Redestad wrote: > > Hi, > > lambdas used in the interaction between JarFile and ZipFile can be > avoided by having ZipFile call a shared secret in JarFile to create a > JarFileEntry when appropriate. This removes an allocation per lookup. > > Bug:https://bugs.openjdk.java.net/browse/JDK-8193066 > Webrev: http://cr.openjdk.java.net/~redestad/8193066/open.00/ > > Testing: tier1-2, verified a small per-op speed-up, significantly so > for negative lookups and during startup/warmup (10-20% > improvements when interpreting) > > Thanks! > > /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR(s): 8244659: Improve ZipFile.getInputStream
4 > avgt3 15.000ms > ZipFileGetInputStream.readAllBytes 4096 > avgt3 20.938 ± 0.577 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 4096 > avgt3 4945.793 ± 0.493B/op > ZipFileGetInputStream.readAllBytes:·gc.count 4096 > avgt3102.000counts > ZipFileGetInputStream.readAllBytes:·gc.time 4096 > avgt3 25.000ms > ZipFileGetInputStream.readAllBytes 16384 > avgt3 51.348 ± 2.600 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 16384 > avgt3 17238.030 ± 3.183B/op > ZipFileGetInputStream.readAllBytes:·gc.count16384 > avgt3144.000counts > ZipFileGetInputStream.readAllBytes:·gc.time 16384 > avgt3 33.000ms > ZipFileGetInputStream.readAllBytes 65536 > avgt3203.082 ± 7.046 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 65536 > avgt3 9035.475 ± 7.426B/op > ZipFileGetInputStream.readAllBytes:·gc.count65536 > avgt3 18.000counts > ZipFileGetInputStream.readAllBytes:·gc.time 65536 > avgt3 5.000ms > ZipFileGetInputStream.readAllBytes 262144 > avgt3801.928 ± 22.474 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 262144 > avgt3 9034.192 ± 0.047B/op > ZipFileGetInputStream.readAllBytes:·gc.count 262144 > avgt3 3.000counts > ZipFileGetInputStream.readAllBytes:·gc.time262144 > avgt3 1.000ms > ZipFileGetInputStream.readAllBytes1048576 > avgt3 3154.747 ± 57.588 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm1048576 > avgt3 9032.194 ± 0.004B/op > ZipFileGetInputStream.readAllBytes:·gc.count 1048576 > avgt3≈ 0counts > > = AFTER 8244659 = > Benchmark (size) > Mode Cnt ScoreError Units > ZipFileGetInputStream.readAllBytes 1024 > avgt313.031 ± 0.452 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 1024 > avgt3 824.311 ± 0.027B/op > ZipFileGetInputStream.readAllBytes:·gc.count 1024 > avgt327.000 counts > ZipFileGetInputStream.readAllBytes:·gc.time 1024 > avgt3 7.000 ms > ZipFileGetInputStream.readAllBytes 4096 > avgt320.018 ± 0.805 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 4096 > avgt3 824.289 ± 0.722B/op > ZipFileGetInputStream.readAllBytes:·gc.count 4096 > avgt315.000 counts > ZipFileGetInputStream.readAllBytes:·gc.time 4096 > avgt3 4.000 ms > ZipFileGetInputStream.readAllBytes 16384 > avgt348.916 ± 1.140 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 16384 > avgt3 824.263 ± 0.008B/op > ZipFileGetInputStream.readAllBytes:·gc.count16384 > avgt3 6.000 counts > ZipFileGetInputStream.readAllBytes:·gc.time 16384 > avgt3 1.000 ms > ZipFileGetInputStream.readAllBytes 65536 > avgt3 192.815 ± 4.102 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 65536 > avgt3 824.012 ± 0.001B/op > ZipFileGetInputStream.readAllBytes:·gc.count65536 > avgt3 ≈ 0 counts > ZipFileGetInputStream.readAllBytes 262144 > avgt3 755.713 ± 42.408 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm 262144 > avgt3 824.047 ± 0.003B/op > ZipFileGetInputStream.readAllBytes:·gc.count 262144 > avgt3 ≈ 0 counts > ZipFileGetInputStream.readAllBytes1048576 > avgt3 2989.236 ± 8.808 us/op > ZipFileGetInputStream.readAllBytes:·gc.alloc.rate.norm1048576 > avgt3 824.184 ± 0.002B/op > ZipFileGetInputStream.readAllBytes:·gc.count 1048576 > avgt3 ≈ 0 counts <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR [15] 8245111: Update doc comments for improved processing by the Standard Doclet
Hi Pavel
> On May 15, 2020, at 12:34 PM, Roger Riggs wrote:
>
> Hi Pavel,
>
> Yes, I think that would be an improvement.
> But its a slippery slope beyond what you originally observed and wanted to
> fix.
I understand why you want to fix this. When you look at the rest of the field
descriptions, that will now show as the only field without a period in the
description.
Not sure what the best answer is.
The rest is fine, this is the only puzzler :-)
Best
Lance
>
> Thanks, Roger
>
>
> On 5/15/20 11:59 AM, Pavel Rappo wrote:
>>> On 15 May 2020, at 16:53, Roger Riggs wrote:
>>>
>>> Hi Pavel,
>>>
>>> No problem with the "with" -> "by" changes.
>>>
>>> javax/naming/NameNotFoundException.java: 55
>>> "initialized" -> "are initialized"
>>>
>>> java/util/jar/Attributes.java:594
>>> The period was in the correct place.
>>> The second sentence is a separate comment about the use (non-use).
>> I considered that initially, but then saw how it was used on the adjacent
>> field:
>>
>> /**
>>* {@code Name} object for {@code Extension-List} manifest attribute
>>* used for the extension mechanism that is no longer supported.
>>*/
>> public static final Name EXTENSION_LIST;
>>
>> Would you suggest making a similar fix here?
>>
>>> "used for the extension mechanism that is no longer supported."
>>>
>>> Would read better as:
>>>"This name is obsolete, the extension mechanism is no longer supported."
>>>
>>> $.02, Roger
>>>
>>>
>>> On 5/15/20 10:00 AM, Daniel Fuchs wrote:
>>>> Hi Pavel,
>>>>
>>>> This looks good to me - but English is not my native language ;-)
>>>>
>>>> cheers,
>>>>
>>>> -- daniel
>>>>
>>>> On 15/05/2020 13:35, Pavel Rappo wrote:
>>>>> Hello,
>>>>>
>>>>> Please review this trivial change for
>>>>> https://bugs.openjdk.java.net/browse/JDK-8245111:
>>>>>
>>>>>http://cr.openjdk.java.net/~prappo/8245111/webrev.00/
>>>>>
>>>>> In addition to fixing the main issue, this includes a blanket change from
>>>>> "followed with" to "followed by" as the latter seemed idiomatic to me.
>>>>>
>>>>> -Pavel
>>>>>
>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.
Looks ok joe > On May 15, 2020, at 1:54 PM, Joe Wang wrote: > > Hi, > > Please review a fix for the compilation warnings. Thanks Roger for the > detailed instructions! If you could verify the fix with the work-in-progress > processor, that would be great too. Regular build and test passed. > > https://bugs.openjdk.java.net/browse/JDK-8244342 > http://cr.openjdk.java.net/~joehw/jdk15/8244342/webrev/ > > Thanks, > Joe <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: JDK-8243454 - Redundant code for executable jar file manifests except SplashScreen-Image
Hi Philipp, I have not looked at this in detail yet, but the changes you are proposing would require a CSR and probably an update to the JAR specification I am not sure of the history as to why this was done, perhaps others might know. I think this would be good to know as part of considering this change/ Best, Lance > On May 17, 2020, at 5:53 AM, Philipp Kunz wrote: > > Hi, > Essentially the same patch as last time [1] but this time with a bug > number [2].The patch does not include the updated UnicodeTest.jar which > I created with > ../jtreg-5.0-b01/bin/jtreg -va -nr -jdk:build/linux-x86_64-server- > release/images/jdk/ -Xshare:off > test/jdk/tools/launcher/UnicodeTest.javacp > JTwork/scratch/UnicodeTest.jar test/jdk/tools/launcher/ > Regards,Philipp > [1] > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064190.html[2 > ] https://bugs.openjdk.java.net/browse/JDK-8243454 > > On Sat, 2020-01-04 at 11:00 +0100, Philipp Kunz wrote: >> Hi, >> When I tried to improve Unicode support in JAR manifests in [1], >> independent of what will happen with that, I found that there are not >> only Manifest and Attributes classes parsing manifests but also some >> c code which parses "SplashScreen-Image" attribute and also used to >> parse some other attributes such as "Main-Class" and others. >> There already are tests for splash screen images but those existing >> ones work with the "-splash" command line option and not with the >> "SplashScreen-Image" manifest attribute. I found "SplashScreen-Image" >> manifest attribute not yet covered with a test and extended the >> existing UnicodeTest accordingly, see attached patch which confirmed >> that the "SplashScreen-Image" manifest attribute already fully >> supports Unicode. >> Support for "JRE-Version" manifest attribute and "-jre-restrict- >> search" and "-jre-no-restrict-search" command line attributes has >> already been removed earlier already and the relevant lines of code >> determining the main class from the manifest when launching have >> already been moved to or near LauncherHelper::getMainClassFromJar in >> earlier versions, apparently leaving them with no use any longer in >> java.c, java.h, manifest_info.h, and parse_manifest.c, I figure. >> Hence, I propose to remove those parts as in the attached patch. >> This leaves manifest_info.h and parse_manifest.c with "SplashScreen- >> Image" as the only attribute parsed there. Certainly it would be a >> different change but anyway it might be worth a consideration to move >> the code opening the splash screen image to LauncherHelper or a >> similar appropriate place in Java which would allow to remove quite a >> number of some lines of c code, provided it could be acceptable to >> show the splash screen image slightly later. >> There is no existing related bug and I didn't find a new one. It >> would be nice to have "SplashScreen-Image" manifest attribute covered >> with a test and there is some potential for cleaning up unused code >> which certainly is not urgent at all and I would not know how >> desirable this really is. Also I'm not sure whether it's better or >> not to add SPLASHSCREEN_IMAGE to Attributes.Name.KNOWN_NAMES. >> Any opinion about to how to proceed with this, if at all or would >> someone sponsor this patch? >> Regards,Philipp >> >> [1] >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/064149.html >> > <20200517-splashscreenimageunicodetestwithoutunicodetestjar.patch> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR [15] 8245231 Javadoc for the readObject methods needs to be updated (8244342 followup)
looks good Joe > On May 18, 2020, at 6:04 PM, Joe Wang wrote: > > As it appears, I was missing a comma in the header as well (see > https://bugs.openjdk.java.net/browse/JDK-8245238) > > The webrev is updated with the comma added (to PredicatedNodeTest.java) > http://cr.openjdk.java.net/~joehw/jdk15/8245231/webrev/ > > Thanks, > Joe > > On 5/18/2020 1:53 PM, Joe Wang wrote: >> Thanks Mark. >> >> -Joe >> >> On 5/18/2020 1:49 PM, mark sheppard wrote: >>> Hi Joe, >>> >>> all good I think. 👍 >>> >>> regards >>> Mark >>> >>> >>> *From:* Joe Wang >>> *Sent:* Monday 18 May 2020 19:36 >>> *To:* mark sheppard ; >>> core-libs-dev@openjdk.java.net >>> *Cc:* Mark Sheppard >>> *Subject:* Re: RFR [15] 8244342: Compilation warnings about unexpected >>> serialization related method signatures. >>> Thanks Mark! I missed that, and I was also 20 min too fast on the checkin >>> :-) >>> >>> Here's a catch-up patch, pls review: >>> http://cr.openjdk.java.net/~joehw/jdk15/8245231/webrev/ >>> >>> -Joe >>> >>> On 5/18/2020 10:21 AM, mark sheppard wrote: >>>> Hi Joe, >>>>do your changes warrant a java doc change for readObject >>>> in each LocPathIterator PredicatedNodeTest, and UnionPathIterator ? >>>> >>>> regards >>>> Mark >>>> >>>> >>>> *From:* core-libs-dev >>>> <mailto:core-libs-dev-boun...@openjdk.java.net> on behalf of Joe Wang >>>> <mailto:huizhe.w...@oracle.com> >>>> *Sent:* Friday 15 May 2020 17:54 >>>> *To:* core-libs-dev@openjdk.java.net >>>> <mailto:core-libs-dev@openjdk.java.net> >>>> <mailto:core-libs-dev@openjdk.java.net> >>>> *Subject:* RFR [15] 8244342: Compilation warnings about unexpected >>>> serialization related method signatures. >>>> Hi, >>>> >>>> Please review a fix for the compilation warnings. Thanks Roger for the >>>> detailed instructions! If you could verify the fix with the >>>> work-in-progress processor, that would be great too. Regular build and >>>> test passed. >>>> >>>> https://bugs.openjdk.java.net/browse/JDK-8244342 >>>> http://cr.openjdk.java.net/~joehw/jdk15/8244342/webrev/ >>>> >>>> Thanks, >>>> Joe >>> >> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: 8239013: java.util.logging.Logger catalog cache keeps strong references to ResourceBundles
Hi Daniel, This looks good to me :-) Best Lance > On May 22, 2020, at 5:33 AM, Daniel Fuchs wrote: > > Hi, > > Please find below a fix for: > > 8239013: java.util.logging.Logger catalog cache keeps strong > references to ResourceBundles > https://bugs.openjdk.java.net/browse/JDK-8239013 > > webrev: > http://cr.openjdk.java.net/~dfuchs/webrev_8239013/webrev.00/ > > java.util.logging.Logger has a `catalog` variable that acts > as a cache and holds a reference to the last resource bundle > that was loaded by the logger. > > This fix replaces the strong reference with a weak reference, > allowing the cached object to be garbage collected even in > cases where a strong reference to the logger remains. > > > best regards, > > -- daniel <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 15 (S): 8245068: Implement Deprecation of RMI Activation
Hi Stuart,
I think this looks good. I will add myself as a reviewer for the CSR.
I would probably create an issue for the release note and create a draft I
believe I was asked for that when I was going through my CSR review for removal
of the Java EE modules and CORBA.
Best
Lance
> On May 26, 2020, at 3:35 PM, Stuart Marks wrote:
>
> Hi all,
>
> Here's the implementation of the recently-posted JEP 385, deprecation of RMI
> Activation for removal.
>
> I'm listing this as S ("small") since conceptually it's fairly small, though
> there are rather a large number of files changed. Essentially the changes are:
>
> - java.rmi.activation package spec: add deprecation warning
> - java.rmi module spec: link to activation package spec
> - java.rmi.activation public classes and interfaces: deprecate
> - various other files: suppress warnings
> - Activation.java: have the rmid tool emit a deprecation warning
> - rmid.properties: localized warning message
>
> Webrev:
>
>http://cr.openjdk.java.net/~smarks/reviews/8245068/webrev.0/
>
> Bug ID:
>
>https://bugs.openjdk.java.net/browse/JDK-8245068
>
> JEP:
>
>http://openjdk.java.net/jeps/385
>
> Thanks,
>
> s'marks
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: 8245867: Logger/bundleLeak/BundleTest.java fails due to "OutOfMemoryError: Java heap space"
Hi Daniel, The change looks reasonable. Thank you for addressing so quickly. Best Lance > On May 28, 2020, at 4:50 AM, Daniel Fuchs wrote: > > Hi, > > Please find an almost trivial fix for: > > 8245867: Logger/bundleLeak/BundleTest.java fails due > to "OutOfMemoryError: Java heap space" > https://bugs.openjdk.java.net/browse/JDK-8245867 > > webrev: > http://cr.openjdk.java.net/~dfuchs/webrev_8245867/webrev.00/ > > My new test for JDK-8239013 has been observed failing > intermittently in OutOfMemory. The test needs to trigger > the clearing of SoftReferences, and does so by eating > up heap memory in order to trigger a GC that will > clear them. > The issue is that the test didn't release the memory > when it no longer needed it, which caused trouble for > the test harness when it tried to clean up after the > test. > > The fix is to use SoftReference to retain the eaten-up > memory (instead of strong references) so that it can > be reclaimed at the time the full GC that clear soft > references is triggered, and also to release the heap > memory as soon as it is no longer needed. > > best regards, > > -- daniel <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 15 (S): 8245068: Implement Deprecation of RMI Activation
Hi Stuart
I think your changes read fine.
> On May 28, 2020, at 1:34 PM, Stuart Marks wrote:
>
> I've updated the webrev a little bit in response to previous comments:
>
>http://cr.openjdk.java.net/~smarks/reviews/8245068/webrev.1/
Thinking about:
-
@deprecated The RMI Activation mechanism has been deprecated, and it may
+ * be removed from a future version.
Perhaps it might be a bit clearer to say “… from a future Java SE version”? I
realize that is different from what is at:
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/Compiler.html
<https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/Compiler.html>,
but I thought it makes it clearer as to what “version” is referring to.
Though that being said, and now looking back at what I did for the CORBA and
Java EE module removal, I used the same wording as you are proposing, so all
good… :-)
>
> I've also drafted a CSR request and a release note. Please review:
>
> CSR:
>
>https://bugs.openjdk.java.net/browse/JDK-8245860
I think the CSR is fine. I might suggest adding a comment to justify “low” for
the compatibility impact for the JCK folks.
I added myself as a reviewer.
>
> Release Note:
>
>https://bugs.openjdk.java.net/browse/JDK-8246021
Looks fine.
Best,
Lance
>
> Thanks,
>
> s'marks
>
>
> On 5/26/20 12:35 PM, Stuart Marks wrote:
>> Hi all,
>> Here's the implementation of the recently-posted JEP 385, deprecation of RMI
>> Activation for removal.
>> I'm listing this as S ("small") since conceptually it's fairly small, though
>> there are rather a large number of files changed. Essentially the changes
>> are:
>> - java.rmi.activation package spec: add deprecation warning
>> - java.rmi module spec: link to activation package spec
>> - java.rmi.activation public classes and interfaces: deprecate
>> - various other files: suppress warnings
>> - Activation.java: have the rmid tool emit a deprecation warning
>> - rmid.properties: localized warning message
>> Webrev:
>> http://cr.openjdk.java.net/~smarks/reviews/8245068/webrev.0/
>> Bug ID:
>> https://bugs.openjdk.java.net/browse/JDK-8245068
>> JEP:
>> http://openjdk.java.net/jeps/385
>> Thanks,
>> s'marks
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR[8245658]: 'Arrays.java has two occurrences of bad unicode constants in Javadoc.'
looks fine Connor > On May 29, 2020, at 5:37 AM, Conor Cleary wrote: > > Hi, > > Could someone please review my webrev for JDK-8245658 'Arrays.java has two > occurrences of bad unicode constants in Javadoc.'? > > In Arrays.java Javadoc, there were two instances of bad Unicode formatting > where the null character constant was incorrectly specified with '\u000' > (another zero is required). This fix displays the correct Unicode constants > in the Javadoc so that outputted docs display '\u'. > > bug: https://bugs.openjdk.java.net/browse/JDK-8245658 > webrev: > http://cr.openjdk.java.net/~pconcannon/ccleary/8245658/webrevs/webrev.01/ > > > Regards, > Conor > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR[8245658]: 'Arrays.java has two occurrences of bad unicode constants in Javadoc.'
Hi Connor, You will want to add the label noreg-doc to the issue before you push for fix Best Lance > On May 29, 2020, at 6:25 AM, Lance Andersen wrote: > > looks fine Connor > >> On May 29, 2020, at 5:37 AM, Conor Cleary wrote: >> >> Hi, >> >> Could someone please review my webrev for JDK-8245658 'Arrays.java has two >> occurrences of bad unicode constants in Javadoc.'? >> >> In Arrays.java Javadoc, there were two instances of bad Unicode formatting >> where the null character constant was incorrectly specified with '\u000' >> (another zero is required). This fix displays the correct Unicode constants >> in the Javadoc so that outputted docs display '\u'. >> >> bug: https://bugs.openjdk.java.net/browse/JDK-8245658 >> webrev: >> http://cr.openjdk.java.net/~pconcannon/ccleary/8245658/webrevs/webrev.01/ >> >> >> Regards, >> Conor >> > > <http://oracle.com/us/design/oracle-email-sig-198324.gif> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> > <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| > Principal Member of Technical Staff | +1.781.442.2037 > Oracle Java Engineering > 1 Network Drive > Burlington, MA 01803 > lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> > > > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR(xs): 8245970: IntStream.html#reduce doc should not mention average
Hi Conor, I believe you want to reference https://bugs.openjdk.java.net/browse/JDK-8242281 <https://bugs.openjdk.java.net/browse/JDK-8242281> as the issue below ,https://bugs.openjdk.java.net/browse/JDK-8245970, is the CSR that was withdrawn. Please add the label noreg-doc prior to finalizing your fix. The change looks fine otherwise. Best Lance > On May 29, 2020, at 5:04 AM, Conor Cleary wrote: > > Hi, > > The method-level documentation of Intstream::reduce(int, IntBinaryOperator) > mentions the average function as a case of reduction even though the function > cannot be used with the reduce method. This might cause confusion, this fix > therefore removes the mention of the average function from the @apiNote. > > bug: https://bugs.openjdk.java.net/browse/JDK-8245970 > webrev: http://cr.openjdk.java.net/~jboes/ccleary/webrevs/8242281/webrev.00/ > > Regards, > Conor > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: 8245867: Logger/bundleLeak/BundleTest.java fails due to "OutOfMemoryError: Java heap space"
Hi Daniel,
Do you still need the SoftReference import given you are no longer using it
with memory.add()?
Best
Lance
> On May 29, 2020, at 11:39 AM, Daniel Fuchs wrote:
>
> Hi,
>
> I have updated the webrev as suggested by David:
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8245867/webrev.01
>
> Unless I hear any objection I'll push that new version if the
> tests come back successful.
>
> best regards,
>
> -- daniel
>
> (tier1 and 2 passed, tier5 still running)
>
> On 29/05/2020 00:36, David Holmes wrote:
>>> webrev:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8245867/webrev.00/
>> I would have expected the fix here to be simply to clear memory ie:
>> } catch (OutOfMemoryError oome) {
>> stop = true;
>> memory = null;
>> System.gc();
>> }
>> Cheers,
>> David
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: [PATCH] Typo in java.util.regex.Pattern
I can sponsor this for you Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPhone > On May 29, 2020, at 10:25 PM, Hong Shao Yang wrote: > > Hi! > > I found a typo which has been long undetected in the documentation. > > In case you think this is worthwhile, I would appreciate it if someone > can sponsor this patch. > > Cheers, > Shao Yang > > = PATCH == > > diff --git a/src/java.base/share/classes/java/util/regex/Pattern.java > b/src/java.base/share/classes/java/util/regex/Pattern.java > index 0d7d0738965..82325d04f25 100644 > --- a/src/java.base/share/classes/java/util/regex/Pattern.java > +++ b/src/java.base/share/classes/java/util/regex/Pattern.java > @@ -760,7 +760,7 @@ import jdk.internal.util.ArraysSupport; > * > * For a more precise description of the behavior of regular expression > * constructs, please see http://www.oreilly.com/catalog/regex3/";> > - * Mastering Regular Expressions, 3nd Edition, Jeffrey E. F. Friedl, > + * Mastering Regular Expressions, 3rd Edition, Jeffrey E. F. Friedl, > * O'Reilly and Associates, 2006. > * > *
Re: [PATCH] Typo in java.util.regex.Pattern
Thank you Naoto, I am building now with both changes and will send a final diff out later along with the bug number! Best Lance > On May 30, 2020, at 2:13 PM, naoto.s...@oracle.com wrote: > > Just noticed that the release year of the 3rd ed. should also be changed to > 2009 in the next line. > > Naoto > > On 5/30/20 4:02 AM, Lance Andersen wrote: >> I can sponsor this for you >> Best >> Lance >> -- >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com >> Sent from my iPhone >>> On May 29, 2020, at 10:25 PM, Hong Shao Yang wrote: >>> >>> Hi! >>> >>> I found a typo which has been long undetected in the documentation. >>> >>> In case you think this is worthwhile, I would appreciate it if someone >>> can sponsor this patch. >>> >>> Cheers, >>> Shao Yang >>> >>> = PATCH == >>> >>> diff --git a/src/java.base/share/classes/java/util/regex/Pattern.java >>> b/src/java.base/share/classes/java/util/regex/Pattern.java >>> index 0d7d0738965..82325d04f25 100644 >>> --- a/src/java.base/share/classes/java/util/regex/Pattern.java >>> +++ b/src/java.base/share/classes/java/util/regex/Pattern.java >>> @@ -760,7 +760,7 @@ import jdk.internal.util.ArraysSupport; >>> * >>> * For a more precise description of the behavior of regular expression >>> * constructs, please see http://www.oreilly.com/catalog/regex3/";> >>> - * Mastering Regular Expressions, 3nd Edition, Jeffrey E. F. Friedl, >>> + * Mastering Regular Expressions, 3rd Edition, Jeffrey E. F. Friedl, >>> * O'Reilly and Associates, 2006. >>> * >>> * <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
RFR: https://bugs.openjdk.java.net/browse/JDK-8246198 (formally: Re: [PATCH] Typo in java.util.regex.Pattern)
I create issue: https://bugs.openjdk.java.net/browse/JDK-8246198 <https://bugs.openjdk.java.net/browse/JDK-8246198> for this typo Here is the updated diff $ hg diff src/java.base/share/classes/java/util/regex/Pattern.java diff -r 968b57610c0f src/java.base/share/classes/java/util/regex/Pattern.java --- a/src/java.base/share/classes/java/util/regex/Pattern.java Sat May 30 10:33:28 2020 +0530 +++ b/src/java.base/share/classes/java/util/regex/Pattern.java Sat May 30 15:02:58 2020 -0400 @@ -760,8 +760,8 @@ * * For a more precise description of the behavior of regular expression * constructs, please see http://www.oreilly.com/catalog/regex3/";> - * Mastering Regular Expressions, 3nd Edition, Jeffrey E. F. Friedl, - * O'Reilly and Associates, 2006. + * Mastering Regular Expressions, 3rd Edition, Jeffrey E. F. Friedl, + * O'Reilly and Associates, 2009. * * * @see java.lang.String#split(String, int) ljanders-mac:open ljanders$ Best Lance > On May 30, 2020, at 2:30 PM, Lance Andersen wrote: > > Thank you Naoto, I am building now with both changes and will send a final > diff out later along with the bug number! > > Best > Lance > >> On May 30, 2020, at 2:13 PM, naoto.s...@oracle.com wrote: >> >> Just noticed that the release year of the 3rd ed. should also be changed to >> 2009 in the next line. >> >> Naoto >> >> On 5/30/20 4:02 AM, Lance Andersen wrote: >>> I can sponsor this for you >>> Best >>> Lance >>> -- >>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >>> Oracle Java Engineering >>> 1 Network Drive >>> Burlington, MA 01803 >>> lance.ander...@oracle.com >>> Sent from my iPhone >>>> On May 29, 2020, at 10:25 PM, Hong Shao Yang wrote: >>>> >>>> Hi! >>>> >>>> I found a typo which has been long undetected in the documentation. >>>> >>>> In case you think this is worthwhile, I would appreciate it if someone >>>> can sponsor this patch. >>>> >>>> Cheers, >>>> Shao Yang >>>> >>>> = PATCH == >>>> >>>> diff --git a/src/java.base/share/classes/java/util/regex/Pattern.java >>>> b/src/java.base/share/classes/java/util/regex/Pattern.java >>>> index 0d7d0738965..82325d04f25 100644 >>>> --- a/src/java.base/share/classes/java/util/regex/Pattern.java >>>> +++ b/src/java.base/share/classes/java/util/regex/Pattern.java >>>> @@ -760,7 +760,7 @@ import jdk.internal.util.ArraysSupport; >>>> * >>>> * For a more precise description of the behavior of regular expression >>>> * constructs, please see http://www.oreilly.com/catalog/regex3/";> >>>> - * Mastering Regular Expressions, 3nd Edition, Jeffrey E. F. >>>> Friedl, >>>> + * Mastering Regular Expressions, 3rd Edition, Jeffrey E. F. >>>> Friedl, >>>> * O'Reilly and Associates, 2006. >>>> * >>>> * > > <http://oracle.com/us/design/oracle-email-sig-198324.gif> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> > <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| > Principal Member of Technical Staff | +1.781.442.2037 > Oracle Java Engineering > 1 Network Drive > Burlington, MA 01803 > lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> > > > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: [PATCH] Typo in java.util.regex.Pattern
All good Naoto Here is the revised diff — $ hg diff src/java.base/share/classes/java/util/regex/Pattern.java diff -r 968b57610c0f src/java.base/share/classes/java/util/regex/Pattern.java --- a/src/java.base/share/classes/java/util/regex/Pattern.java Sat May 30 10:33:28 2020 +0530 +++ b/src/java.base/share/classes/java/util/regex/Pattern.java Sat May 30 18:39:02 2020 -0400 @@ -760,7 +760,7 @@ * * For a more precise description of the behavior of regular expression * constructs, please see http://www.oreilly.com/catalog/regex3/";> - * Mastering Regular Expressions, 3nd Edition, Jeffrey E. F. Friedl, + * Mastering Regular Expressions, 3rd Edition, Jeffrey E. F. Friedl, * O'Reilly and Associates, 2006. * * ljanders-mac:open ljanders$ --- Best Lance > On May 30, 2020, at 6:35 PM, naoto.s...@oracle.com wrote: > > Thanks, Pavel. I think the year stays 2006 then. > Sorry, Lance, for the false alarm. > > Naoto > > On 5/30/20 2:34 PM, Pavel Rappo wrote: >> I'm holding a copy in my hands right now. I can see "ISBN: >> 978-0-596-52812-6" written on the back of the book. Searching that number in >> the database yields the following record: >> Mastering Regular Expressions >> ISBN-13: 9780596528126 >> ISBN-10: 0596528124 >> Author: Friedl, Jeffrey E. F. >> Edition: Third >> Binding: Paperback >> Publisher: O'Reilly Media >> Published: August 2006 >> I think it's reasonable to assume that a changeset pushed in November 2006 >> was referring to a book published no later than that. Certainly, not in >> August 2009. Which is what the page refers to >> http://shop.oreilly.com/product/9780596528126.do Is it a newer print of the >> same edition? It does looks like it. >> I would use the original patch as is. >> -Pavel >>> On 30 May 2020, at 19:13, naoto.s...@oracle.com wrote: >>> >>> Just noticed that the release year of the 3rd ed. should also be changed to >>> 2009 in the next line. >>> >>> Naoto >>> >>> On 5/30/20 4:02 AM, Lance Andersen wrote: >>>> I can sponsor this for you >>>> Best >>>> Lance >>>> -- >>>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >>>> Oracle Java Engineering >>>> 1 Network Drive >>>> Burlington, MA 01803 >>>> lance.ander...@oracle.com >>>> Sent from my iPhone >>>>> On May 29, 2020, at 10:25 PM, Hong Shao Yang wrote: >>>>> >>>>> Hi! >>>>> >>>>> I found a typo which has been long undetected in the documentation. >>>>> >>>>> In case you think this is worthwhile, I would appreciate it if someone >>>>> can sponsor this patch. >>>>> >>>>> Cheers, >>>>> Shao Yang >>>>> >>>>> = PATCH == >>>>> >>>>> diff --git a/src/java.base/share/classes/java/util/regex/Pattern.java >>>>> b/src/java.base/share/classes/java/util/regex/Pattern.java >>>>> index 0d7d0738965..82325d04f25 100644 >>>>> --- a/src/java.base/share/classes/java/util/regex/Pattern.java >>>>> +++ b/src/java.base/share/classes/java/util/regex/Pattern.java >>>>> @@ -760,7 +760,7 @@ import jdk.internal.util.ArraysSupport; >>>>> * >>>>> * For a more precise description of the behavior of regular >>>>> expression >>>>> * constructs, please see >>>> href="http://www.oreilly.com/catalog/regex3/";> >>>>> - * Mastering Regular Expressions, 3nd Edition, Jeffrey E. F. >>>>> Friedl, >>>>> + * Mastering Regular Expressions, 3rd Edition, Jeffrey E. F. >>>>> Friedl, >>>>> * O'Reilly and Associates, 2006. >>>>> * >>>>> * <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"
Hi Naoto, Looks reasonable to me :-) > On Jun 1, 2020, at 3:31 PM, naoto.s...@oracle.com wrote: > > Hello, > > Please review the fix to the following issue: > > https://bugs.openjdk.java.net/browse/JDK-8246261 > > The proposed changeset is located at: > > https://cr.openjdk.java.net/~naoto/8246261/webrev.00/ > > The test case compares two LocalTime objects, created with > LocalTime.now(Clock/ZoneId). So inherently those two objects could have > different times. The test tries to compare them 100 times for the exact > match, and if not, then falls back to compare those times by truncating > nanoseconds. The failure could occur when those two LocalTimes are around the > whole second, e.g., expected == 18:14:22.99 and test == 18:14:23.01. > To fix this, check the difference of those objects and ensure it is less than > a second. > > Similar test cases exist in TCKLocalDateTime.java and TCKZonedDateTime.java > so they should also be fixed. It is ok to leave the similar test case in > TCKLocalDate.java, as multiple tries do exact match. > > Naoto <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"
The change looks OK also with the loop removed. > On Jun 2, 2020, at 12:13 AM, naoto.s...@oracle.com wrote: > > Updated webrev: http://cr.openjdk.java.net/~naoto/8246261/webrev.01/ > > Naoto > > On 6/1/20 6:30 PM, naoto.s...@oracle.com wrote: >> Hi Joe, >> In fact, this bug was possibly revealed by the fix to 8242504, where the >> system clock precision is now nanoseconds. Before that, it used to be >> millisecond precision, so the first try for the exact match succeeded for >> most of the cases. Even with the nano precision fix, most of the cases the >> test exits with exact match in the loop. But you are right, exact match or >> not does not matter in this test case, so I think we can just eliminate >> these exact match try loops. I will remove them and do some sniff testing on >> it. >> Naoto >> On 6/1/20 5:58 PM, Joe Wang wrote: >>> Hi Naoto, >>> >>> The patch looks good to fix the failure. I'm just curious whether the >>> 100-time comparison is necessary because of the existence of this assertion >>> outside the loop that allowed the test to pass if the different was within >>> a certain period of time. None of the tests had commented on the purpose of >>> the test, it looks like it's testing the assertion that (for the now >>> method) "This will query the system clock to obtain the current time." The >>> 100-loop therefore was a compromise for lack of a better way to prove that. >>> I agree with what you said that "inherently those two objects could have >>> different times". The outside-loop assertion therefore makes better sense, >>> and the loop was kind of just wasting time to me (I mean you could get >>> lucky to have the two returning the same time down to a nanosecond, but >>> that didn't make the test better than just the out-of-loop assertion. >>> >>> my 2 cents >>> >>> -Joe >>> >>> On 6/1/2020 12:31 PM, naoto.s...@oracle.com wrote: >>>> Hello, >>>> >>>> Please review the fix to the following issue: >>>> >>>> https://bugs.openjdk.java.net/browse/JDK-8246261 >>>> >>>> The proposed changeset is located at: >>>> >>>> https://cr.openjdk.java.net/~naoto/8246261/webrev.00/ >>>> >>>> The test case compares two LocalTime objects, created with >>>> LocalTime.now(Clock/ZoneId). So inherently those two objects could have >>>> different times. The test tries to compare them 100 times for the exact >>>> match, and if not, then falls back to compare those times by truncating >>>> nanoseconds. The failure could occur when those two LocalTimes are around >>>> the whole second, e.g., expected == 18:14:22.99 and test == >>>> 18:14:23.01. To fix this, check the difference of those objects and >>>> ensure it is less than a second. >>>> >>>> Similar test cases exist in TCKLocalDateTime.java and >>>> TCKZonedDateTime.java so they should also be fixed. It is ok to leave the >>>> similar test case in TCKLocalDate.java, as multiple tries do exact match. >>>> >>>> Naoto >>> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8246451: Reduce overhead of normalizing file paths with trailing slash
Hi Claes, This looks OK :-) Best Lance > On Jun 3, 2020, at 7:54 AM, Claes Redestad wrote: > > Hi, > > I'd like to fix a bug in the microbenchmark I pushed for JDK-8246338 > where the FileOpen.trailingSlash variant doesn't do what it intends to. > > There's also a profitable low-hanging optimization down that path to > substring rather than take the detour via StringBuilder[1] > > Bug:https://bugs.openjdk.java.net/browse/JDK-8246451 > Webrev: http://cr.openjdk.java.net/~redestad/8246451/open.00/ > > Testing: tier1 > > Thanks! > > /Claes > > [1] > BenchmarkMode Cnt Score > Error Units > FileOpen.trailingSlash avgt5 0.054 ± > 0.010 us/op > FileOpen.trailingSlash:·gc.alloc.rate.norm avgt5 160.013 ± > 0.003B/op > > FileOpen.trailingSlash avgt5 0.036 ± > 0.003 us/op > FileOpen.trailingSlash:·gc.alloc.rate.norm avgt5 96.008 ± > 0.002B/op <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: contributing to JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end
Hi Raffaello, If you are interested in providing a fix, you are more than welcome to do so. Please include a test which validates your fix. Best lance > On Jun 8, 2020, at 3:45 PM, Raffaello Giulietti > wrote: > > Raffaello <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: contributing to JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end
Hi Raffaello, This bug is not currently being worked on so feel free to provide a fix and test Best Lance > On Jun 9, 2020, at 3:20 AM, Raffaello Giulietti > wrote: > > Hi Lance, > > before working on a fix, I just wanted to make sure that I'm not interfering > with existing efforts. Thus, I don't have a fix, yet. > > I'll be using the example provided in the bug report as a basic test. > > I'll show up here once the fix is ready. > > > Greetings > Raffaello > > > > On 2020-06-08 22:34, Lance Andersen wrote: >> Hi Raffaello, >> If you are interested in providing a fix, you are more than welcome to do >> so. Please include a test which validates your fix. >> Best >> lance >>> On Jun 8, 2020, at 3:45 PM, Raffaello Giulietti >>> mailto:raffaello.giulie...@gmail.com>> >>> wrote: >>> >>> Raffaello >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| >> Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR [15] 8247115: Fix typos in java.lang.invoke and java.lang
Hi Pavel, The changes look good Best Lance > On Jun 9, 2020, at 6:42 AM, Pavel Rappo wrote: > > Hello, > > Please review the change for https://bugs.openjdk.java.net/browse/JDK-8247115 > > http://cr.openjdk.java.net/~prappo/8247115/webrev.00/ > > The patch fixes 11 typos in doc comments and 2 typos in code. The typos in > code are in exception messages. Testing results are pending. > > -Pavel > > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR [15/java.xml] 8237456: Transform filtered through SAX filter mishandles character entities
Hi Joe, This looks good. For your test, Intellij typically suggests using a static import for Assert: —— import static org.testng.Assert.*; — No need to change but typically which get suggested when you run Inspect Code Best Lance > On Jun 9, 2020, at 1:54 AM, Joe Wang wrote: > > Hi, > > Please review a fix to a regression that resulted in invalid output in a > transform that contains entity references. The regression was caused by a > JDK9 patch [1]. Adding back the conditional checks fixed the issue. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8237456 > webrevs: http://cr.openjdk.java.net/~joehw/jdk15/8237456/webrev/ > > [1] https://bugs.openjdk.java.net/browse/JDK-8087303 > > Thanks, > Joe > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8246129: ZIP entries created for DOS epoch include local timezone metadata
Hi Martin, I believe the fix keeps compatibility with the previous fixes in this area(see 4759491 8015666 and 8073497). If you have an alternative suggestion please let us know. > On Jun 9, 2020, at 11:51 AM, Martin Buchholz wrote: > > not really a review, but cautions: > > I was afraid that might be hard to fix because DOSTIME_BEFORE_1980 was > used as a special value indicating that the real time was < 1980 and > stored in an extra field. But I guess we now don't have any code that > makes assumptions based only on DOSTIME_BEFORE_1980? > Keep in mind that dostime is local date time, and there's always risk > in the zip code of confusing time-zoned date time with un-zoned. Sherman added set/getTimeLocal via JDK-8075526 which can help I believe. Best, Lance > On Tue, Jun 9, 2020 at 8:24 AM Claes Redestad > wrote: >> >> Hi, >> >> this patch addresses a corner case where extra time stamp information >> is added when using start of the DOS time epoch (1980-01-01 00:00:00) >> as a dummy timestamp for entries >> >> Bug:https://bugs.openjdk.java.net/browse/JDK-8246129 >> Webrev: http://cr.openjdk.java.net/~redestad/8246129/open.00/ >> >> Testing: tier1-2 >> >> Thanks! >> >> /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
RFR 8207936: TestZipFile.java can fail with an OOM error
Hi, Please review the fix for JDK-8207936, which addresses the OOM error when running the manual test TestZipFile.java. The fix also removes the call to "getMetaInfEntryNames” as this method was removed as part of the performance work done in JDK-8244624. The webrev can be found at: http://cr.openjdk.java.net/~lancea/8207936/webrev.00/ Best Lance <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8218021: jarsigner strips the execute permission when signing a .zip file
Hi Sean, I think your changes look fine so all good FMPOV. Best Lance > On Jun 12, 2020, at 6:21 AM, Seán Coffey wrote: > > Hi, > > I'd like to reboot this jarsigner enhancement request[1]. I've removed the > problem references to zip file name extensions. Instead, there's a new JDK > implementation specific jarsigner option: -keepposixperms > > https://bugs.openjdk.java.net/browse/JDK-8218021 > https://cr.openjdk.java.net/~coffeys/webrev.8218021.v2/webrev/ > > regards, > Sean. > > [1] > http://mail.openjdk.java.net/pipermail/security-dev/2020-January/021141.html > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8245448: Remove minimum 4 digit requirement from Year.parse()
Hi Naoto, This looks fine and looks like you are set with reviewers on your CSR Best Lance > On Jun 12, 2020, at 12:31 PM, naoto.s...@oracle.com wrote: > > Hello, > > Please review the fix to the following issue: > > https://bugs.openjdk.java.net/browse/JDK-8245448 > > The proposed changeset and its CSR are located at: > > https://cr.openjdk.java.net/~naoto/8245448/webrev.00/ > https://bugs.openjdk.java.net/browse/JDK-8246623 > > Naoto <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8247782: typos in java.math
Looks fine. Please add the label noreg-doc to make the automatic verification happy :-) Best Lance > On Jun 17, 2020, at 2:44 PM, Martin Buchholz wrote: > > 8247782: typos in java.math > https://cr.openjdk.java.net/~martin/webrevs/jdk/math-spelling/ > https://bugs.openjdk.java.net/browse/JDK-8247782 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8218021: jarsigner strips the execute permission when signing a .zip file
HI Sean, Looks OK based on our exchanges. Thank you for your time on this one! Best Lance > On Jun 22, 2020, at 7:22 AM, Seán Coffey wrote: > > Thanks Lance. > > I've updated the patch with some extra offline feedback from yourself and Max. > A new warning is printed with use of the new flag. A warning is also printed > when file posix permissions are detected on resources being signed. Test > updated for that also. > > https://cr.openjdk.java.net/~coffeys/webrev.8218021.v3/webrev/ > <https://cr.openjdk.java.net/~coffeys/webrev.8218021.v3/webrev/> > regards, > Sean. > > On 12/06/2020 17:05, Lance Andersen wrote: >> Hi Sean, >> >> I think your changes look fine so all good FMPOV. >> >> Best >> Lance >> >>> On Jun 12, 2020, at 6:21 AM, Seán Coffey >> <mailto:sean.cof...@oracle.com>> wrote: >>> >>> Hi, >>> >>> I'd like to reboot this jarsigner enhancement request[1]. I've removed the >>> problem references to zip file name extensions. Instead, there's a new JDK >>> implementation specific jarsigner option: -keepposixperms >>> >>> https://bugs.openjdk.java.net/browse/JDK-8218021 >>> <https://bugs.openjdk.java.net/browse/JDK-8218021> >>> https://cr.openjdk.java.net/~coffeys/webrev.8218021.v2/webrev/ >>> <https://cr.openjdk.java.net/~coffeys/webrev.8218021.v2/webrev/> >>> >>> regards, >>> Sean. >>> >>> [1] >>> http://mail.openjdk.java.net/pipermail/security-dev/2020-January/021141.html >>> >>> <http://mail.openjdk.java.net/pipermail/security-dev/2020-January/021141.html> >>> >> >> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| >> Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> >> >> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 8248184: AMPM_OF_DAY doc fix in ChronoField
+1
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
> On Jun 23, 2020, at 5:44 PM, naoto.s...@oracle.com wrote:
>
> Hi,
>
> Please review this small doc fix for the following issue:
>
> https://bugs.openjdk.java.net/browse/JDK-8248184
>
> The proposed patch is here:
>
> --- old/src/java.base/share/classes/java/time/temporal/ChronoField.java
> 2020-06-23 14:43:43.0 -0700
> +++ new/src/java.base/share/classes/java/time/temporal/ChronoField.java
> 2020-06-23 14:43:43.0 -0700
> @@ -270,7 +270,7 @@
> * The value is validated from 0 to 11 in strict and smart mode.
> * In lenient mode the value is not validated. It is combined with
> * {@code AMPM_OF_DAY} to form {@code HOUR_OF_DAY} by multiplying
> - * the {AMPM_OF_DAY} value by 12.
> + * the {@code AMPM_OF_DAY} value by 12.
> *
> * See {@link #CLOCK_HOUR_OF_AMPM} for the related field that counts
> hours from 1 to 12.
> */
> @@ -334,7 +334,7 @@
> * The value is validated from 0 to 1 in strict and smart mode.
> * In lenient mode the value is not validated. It is combined with
> * {@code HOUR_OF_AMPM} to form {@code HOUR_OF_DAY} by multiplying
> - * the {AMPM_OF_DAY} value by 12.
> + * the {@code AMPM_OF_DAY} value by 12.
> */
> AMPM_OF_DAY("AmPmOfDay", HALF_DAYS, DAYS, ValueRange.of(0, 1),
> "dayperiod"),
>
> Naoto
RFR 8248412: test/jdk/java/sql/testng/test/sql/DriverManagerPermissionsTests.java can fail
Hi all, Please review this patch for https://bugs.openjdk.java.net/browse/JDK-8248412 <https://bugs.openjdk.java.net/browse/JDK-8248412> where DriverManagerPermissionsTests.java can occasionally fail in a mach5 run specifying -test open/test/jdk/:jdk_core,closed/test/jdk/:jdk_core The webrev can be found at: http://cr.openjdk.java.net/~lancea/8248412/webrev.00/ <http://cr.openjdk.java.net/~lancea/8248412/webrev.00/> and change is trivial as the test will now run using othervm mode Best Lance <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: request for review JDK-8211290
Hi Ivan, Looks fine to me also outside of the one extra line that Roger caught. Best Lance > On Jun 30, 2020, at 8:38 AM, Ivan Sipka wrote: > > Hi all, > > kind reminder for RFR for JDK-8211974. > > thank you, > > - Original Message - > From: ivan.si...@oracle.com > To: core-libs-dev@openjdk.java.net > Cc: igor.ignat...@oracle.com > Sent: Thursday, June 4, 2020 7:40:27 PM GMT +00:00 GMT Britain, Ireland, > Portugal > Subject: request for review JDK-8211290 > > Hi all, > > please review the following changeset: > http://cr.openjdk.java.net/~iignatyev/isipka/8211974/webrev.01/index.html > for the JBS issue https://bugs.openjdk.java.net/browse/JDK-8211974 > > which moves the files: > > open/test/jdk/lib/testlibrary/java/util/jar/Compiler.java > open/test/jdk/lib/testlibrary/java/util/jar/JarBuilder.java > > to files: > > open/test/lib/jdk/test/lib/util/JarBuilder.java > open/test/lib/jdk/test/lib/compiler/Compiler.java > > and changes the relevant jtreg specification tags in dependent tests: > > open/test/jdk/sun/net/www/protocol/jar/MultiReleaseJarURLConnection.java > open/test/jdk/jdk/nio/zipfs/jarfs/JFSTester.java > open/test/jdk/jdk/nio/zipfs/jarfs/MultiReleaseJarTest.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarHttpProperties.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarProperties.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarSecurity.java > open/test/jdk/java/util/jar/JarFile/JarBacktickManifest.java > open/test/jdk/java/net/URL/JarHandlerPkgPrefix/JarHandlerPkgPrefix.java > > and adds relevant package imports to dependent file: > > open/test/jdk/lib/testlibrary/java/util/jar/CreateMultiReleaseTestJars.java > > jtreg command to run all dependent tests from repository root > > jtreg -verbose:summary -ea -esa -a -conc:9 -jdk:build/linux-x64/jdk/ > open/test/jdk/sun/net/www/protocol/jar/MultiReleaseJarURLConnection.java > open/test/jdk/jdk/nio/zipfs/jarfs/JFSTester.java > open/test/jdk/jdk/nio/zipfs/jarfs/MultiReleaseJarTest.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarHttpProperties.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarProperties.java > open/test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarSecurity.java > open/test/jdk/java/util/jar/JarFile/JarBacktickManifest.java > open/test/jdk/java/net/URL/JarHandlerPkgPrefix/JarHandlerPkgPrefix.java > > thank you, <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: 8218021: Have jarsigner preserve posix permission attributes
Hi Sean, I think the changes look good including the proposed tweaks to the message suggested by Alan. Best Lance > On Jul 2, 2020, at 4:10 AM, Seán Coffey wrote: > > Thanks for the review Alan. I'm in contact with Max already about possible > follow up enhancements in this area. It would be worked via a follow on JBS > record. > > Regarding the error message, I'm fine with your suggestion. We can go with > this then: > "POSIX file permission attributes detected. These attributes are ignored when > signing and are not protected by the signature." > > regards, > Sean. > > On 02/07/2020 08:59, Alan Bateman wrote: >> On 30/06/2020 14:51, Seán Coffey wrote: >>> >>> : >>> >>> During the CSR review, a suggestion was made to have jarsigner preserve >>> such attributes by default. Warnings about these attributes will also be >>> added during signing and verify operations (if detected). >>> >> Yes, signing should be additive so the original proposal to drop information >> from the UNIX extra block would be surprising. The intersection of those >> using zip/other tools to create zip files and then signing them with >> jarsigner is probably small but it would still be confusing for signing to >> loose information. Having jarsigner refuse to sign these zip files by >> default, with an option to override, would be a reasonable approach. The >> current proposal to printing a warning seems okay too. >> >> I've skimmed through webrev.8218021.v5 which has this warning: >> >> "POSIX file permission attributes detected. Note that these attributes are >> unsigned and not protected by the signature." >> >> I realize you've agreed this with the other Reviewers but I think that "Note >> that these attributes are unsigned ..." is confusing as it could be >> interpreted to mean that they have to be signed by some other means, or even >> that the warning is because they are using unsigned values. >> >> It might be better to tweak the second part to make it a bit clearer, up to >> you but something like "These attributes are ignored when signing and are >> not protected by the signature". >> >> -Alan Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end
;>>>> +}
>>>>> +Objects.checkFromIndexSize(off, len, b.length);
>>>>> +if (len == 0) {
>>>>> +return 0;
>>>>> }
>>>>> -bits = 0;
>>>>> -while (len > 0) {
>>>>> -int v = is.read();
>>>>> -if (v == -1) {
>>>>> -return eof(b, off, len, oldOff);
>>>>> +
>>>>> +/*
>>>>> +Rather than keeping 2 running vars (e.g., off and len), we
>>>>> only
>>>>> +keep one (pos), while definitely fixing the boundaries of
>>>>> the range
>>>>> +[off, limit).
>>>>> +
>>>>> +Note that limit can overflow to Integer.MIN_VALUE. However,
>>>>> as long
>>>>> +as comparisons with pos are done as coded, there's no harm.
>>>>> +
>>>>> +In addition, limit - off (= len) is used from here on, so the
>>>>> +location for len can be reallocated to other vars (e.g.,
>>>>> limit).
>>>>> + */
>>>>> +int pos = off;
>>>>> +int limit = off + len;
>>>>> +if (eos) {
>>>>> +return leftovers(b, off, pos, limit);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> +leftovers from previous invocation; here, wpos = 0.
>>>>> +There can be at most 2 leftover bytes (rpos <= 16).
>>>>> +Further, the buffer b has at least one free place.
>>>>> +
>>>>> +The logic could be coded as a loop, (as in method
>>>>> leftovers())
>>>>> +but the explicit "unrolling" makes it possible to generate
>>>>> better
>>>>> +byte extraction code.
>>>>> + */
>>>>> +if (rpos == 16) {
>>>>> +b[pos++] = (byte) (bits >> 8);
>>>>> +rpos = 8;
>>>>> +if (pos == limit) {
>>>>> +return limit - off;
>>>>> }
>>>>> -if ((v = base64[v]) < 0) {
>>>>> -if (v == -2) { // padding byte(s)
>>>>> -return padding(b, off, len, oldOff);
>>>>> -}
>>>>> -if (v == -1) {
>>>>> -if (!isMIME)
>>>>> -throw new IOException("Illegal base64
>>>>> character " +
>>>>> -Integer.toString(v, 16));
>>>>> -continue;// skip if for rfc2045
>>>>> -}
>>>>> -// neve be here
>>>>> -}
>>>>> -bits |= (v << nextin);
>>>>> -if (nextin == 0) {
>>>>> -nextin = 18; // clear for next in
>>>>> -b[off++] = (byte)(bits >> 16);
>>>>> -if (len == 1) {
>>>>> -nextout = 8;// 2 bytes left in bits
>>>>> -break;
>>>>> -}
>>>>> -b[off++] = (byte)(bits >> 8);
>>>>> -if (len == 2) {
>>>>> -nextout = 0;// 1 byte left in bits
>>>>> -break;
>>>>> -}
>>>>> -b[off++] = (byte)bits;
>>>>> -len -= 3;
>>>>> -bits = 0;
>>>>> -} else {
>>>>> -nextin -= 6;
>>>>> +}
>>>>> +if (rpos == 8) {
>>>>> +b[pos++] = (byte) bits;
>>>>> +rpos = 0;
>>>>> +if (pos == limit) {
>>>>> +return limit - off;
>>>>> }
>>>>> }
>>>>> -return off - oldOff;
>>>>> + 8222187
>>>>> +bits = 0;
>>>>> +wpos = 24;
>>>>> +for (;;) {
>>>>> +// Here, pos != limit
>>>>> +int i = is.read();
>>>>> +if (i < 0) {
>>>>> +return eos(b, off, pos, limit);
>>>>> +}
>>>>> +int v = base64[i];
>>>>> +if (v < 0) {
>>>>> +/*
>>>>> +i not in alphabet, thus
>>>>> +v = -2: i is '=', the padding
>>>>> +v = -1: i is something else, e.g., CR or LF
>>>>> + */
>>>>> +if (v == -1) {
>>>>> +if (isMIME) {
>>>>> +continue;
>>>>> +}
>>>>> +throw new IOException("Illegal base64 byte 0x" +
>>>>> +Integer.toHexString(i));
>>>>> +}
>>>>> +return padding(b, off, pos, limit);
>>>>> +}
>>>>> +bits |= (v << (wpos -= 6));
>>>>> +if (wpos != 0) {
>>>>> +continue;
>>>>> +}
>>>>> +if (limit - pos >= 3) {
>>>>> +// frequently taken fast path, no need to track rpos
>>>>> +b[pos++] = (byte) (bits >> 16);
>>>>> +b[pos++] = (byte) (bits >> 8);
>>>>> +b[pos++] = (byte) bits;
>>>>> +bits = 0;
>>>>> +wpos = 24;
>>>>> +if (pos == limit) {
>>>>> +return limit - off;
>>>>> +}
>>>>> +continue;
>>>>> +}
>>>>> +/*
>>>>> +Here, buffer b has either 1 or 2 free places, that is,
>>>>> +limit - pos = 1 or limit - pos = 2.
>>>>> +
>>>>> +As above, this could be coded as a loop. But since the
>>>>> +shift lengths are explicit multiples of 8, better code
>>>>> can be
>>>>> +probably generated.
>>>>> + */
>>>>> +b[pos++] = (byte) (bits >> 16);
>>>>> +if (pos == limit) {
>>>>> +rpos = 16;
>>>>> +return limit - off;
>>>>> +}
>>>>> +b[pos++] = (byte) (bits >> 8);
>>>>> +// Here, pos = limit, no need for an if.
>>>>> +rpos = 8;
>>>>> +return limit - off;
>>>>> +}
>>>>> }
>>>>>
>>>>> @Override
>>>>> diff --git a/test/jdk/java/util/Base64/TestBase64.java
>>>>> b/test/jdk/java/util/Base64/TestBase64.java
>>>>> --- a/test/jdk/java/util/Base64/TestBase64.java
>>>>> +++ b/test/jdk/java/util/Base64/TestBase64.java
>>>>> @@ -144,6 +144,10 @@
>>>>> testDecoderKeepsAbstinence(Base64.getDecoder());
>>>>> testDecoderKeepsAbstinence(Base64.getUrlDecoder());
>>>>> testDecoderKeepsAbstinence(Base64.getMimeDecoder());
>>>>> +
>>>>> +// tests patch addressing JDK-8222187
>>>>> +// https://bugs.openjdk.java.net/browse/JDK-8222187
>>>>> +testJDK_8222187();
>>>>> }
>>>>>
>>>>> private static void test(Base64.Encoder enc, Base64.Decoder dec,
>>>>> @@ -607,4 +611,26 @@
>>>>> }
>>>>> }
>>>>> }
>>>>> +
>>>>> +private static void testJDK_8222187() throws Throwable {
>>>>> +byte[] orig = "12345678".getBytes("US-ASCII");
>>>>> +byte[] encoded = Base64.getEncoder().encode(orig);
>>>>> +// decode using different buffer sizes
>>>>> +for (int bufferSize = 1; bufferSize <= encoded.length + 1;
>>>>> bufferSize++) {
>>>>> +try (
>>>>> +InputStream in = Base64.getDecoder().wrap(
>>>>> +new ByteArrayInputStream(encoded));
>>>>> +ByteArrayOutputStream baos = new
>>>>> ByteArrayOutputStream();
>>>>> +) {
>>>>> +byte[] buffer = new byte[bufferSize];
>>>>> +int read;
>>>>> +while ((read = in.read(buffer, 0, bufferSize)) >= 0) {
>>>>> +baos.write(buffer, 0, read);
>>>>> +}
>>>>> +// compare result, output info if lengths do not match
>>>>> +byte[] decoded = baos.toByteArray();
>>>>> +checkEqual(decoded, orig, "Base64 stream decoding
>>>>> failed!");
>>>>> +}
>>>>> +}
>>>>> +}
>>>>> }
>>>>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
Hi Vipin,
Apologies for the delay.
After looking at the bug, which is over 14 years old, the SCCS history of
Attributes.java, I am reluctant to suggest we move forward with your proposed
change.
The key for an Attributes map entry should be an Attributes.Name object (see
Attributes::put). Unfortunately your proposed fix introduces a behavioral
change and could possibly break existing applications.
A behavioral change to existing public methods would require approval via a
CSR and would require more compressive testing. I took a quick scan of the
JCK tests and of the JTReg tests and I believe your change would cause some of
the existing tests to fail.
From my perspective, it would be better to clarify the Attributes javadoc to
make it clearer that an Attributes.Name object is required (which I believe
has not changed since the Attributes class was added to Java SE).
Best
Lance
> On Jul 1, 2020, at 12:42 AM, Vipin Mv1 wrote:
>
> Hi,
>
> A gentle reminder to please review this patch.
>
> Thanks & Regards
> Vipin MV
>
>
>
>
>
> -Vipin Mv1/India/IBM wrote: -
> To: core-libs-dev@openjdk.java.net
> From: Vipin Mv1/India/IBM
> Date: 06/15/2020 11:52AM
> Subject: Re: RFR 6470126 java.util.jar.Attributes#containsKey fails with
> Strings
>
> Hi,
>
> I have addressed the review comments and the patch has been uploaded here:
>
> http://cr.openjdk.java.net/~vtewari/6470126/webrev/index.html
>
> Please let me know your suggestions.
>
> Thanks & Regards
> Vipin MV
>
>
>
> -Vipin Mv1/India/IBM wrote: -
> To: core-libs-dev@openjdk.java.net
> From: Vipin Mv1/India/IBM
> Date: 05/11/2020 05:00PM
> Subject: RFR 6470126 java.util.jar.Attributes#containsKey fails with Strings
>
> Hi,
>
> Please review the fix for the following issue.
>
> https://bugs.openjdk.java.net/browse/JDK-6470126
>
>
> diff -r 53568400fec3 src/java.base/share/classes/java/util/jar/Attributes.java
> --- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Mar 26
> 15:26:51 2020 +
> +++ b/src/java.base/share/classes/java/util/jar/Attributes.java Mon May 11
> 15:00:01 2020 +0530
> @@ -205,7 +205,10 @@
> * @return true if this Map contains the specified attribute name
> */
> public boolean containsKey(Object name) {
> -return map.containsKey(name);
> +if(String.class.isInstance(name))
> +return map.containsKey(Name.of((String)name));
> +else
> +return map.containsKey(name);
> }
>
> /**
>
> Thanks & Regards
> Vipin Menon
>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
RFR 824767: Incorrect class name displayed in DriverManager trace output
Hi all,
Please review this trivial fix for
https://bugs.openjdk.java.net/browse/JDK-8247677
<https://bugs.openjdk.java.net/browse/JDK-8247677>, which addresses a couple
of typos where the driver class name was not included in the trace output from
DriverManager.
———
$ hg diff
diff -r e92a076bc6a5 src/java.sql/share/classes/java/sql/DriverManager.java
--- a/src/java.sql/share/classes/java/sql/DriverManager.javaFri Jun 26
13:37:43 2020 -0400
+++ b/src/java.sql/share/classes/java/sql/DriverManager.javaTue Jul 14
10:15:42 2020 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -449,7 +449,7 @@
if (isDriverAllowed(aDriver.driver, callerClass)) {
result.add(aDriver.driver);
} else {
-println("skipping: " + aDriver.getClass().getName());
+println("skipping: " +
aDriver.driver.getClass().getName());
}
}
return result;
@@ -687,7 +687,7 @@
}
} else {
-println("skipping: " + aDriver.getClass().getName());
+println("skipping: " +
aDriver.driver.getClass().getName());
}
}
——
Best,
Lance
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end
continue;
> +}
> +throw new IOException("Illegal base64 character 0x" +
> +Integer.toHexString(i));
> +}
> +return padding(b, off, pos, limit);
> +}
> +wpos -= 6;
> +bits |= v << wpos;
> +if (wpos != 0) {
> +continue;
> +}
> +if (limit - pos >= 3) {
> +/* frequently taken fast path, no need to track rpos */
> +b[pos++] = (byte) (bits >> 16);
> +b[pos++] = (byte) (bits >> 8);
> +b[pos++] = (byte) bits;
> +bits = 0;
> +wpos = 24;
> +if (pos == limit) {
> +return len;
> +}
> +continue;
> +}
> +
> +/* b has either 1 or 2 free places */
> +b[pos++] = (byte) (bits >> 16);
> +if (pos == limit) {
> +rpos = 16;
> +return len;
> +}
> +b[pos++] = (byte) (bits >> 8);
> +/* pos == limit, no need for an if */
> +rpos = 8;
> +return len;
> +}
> }
>
> @Override
> diff --git a/test/jdk/java/util/Base64/TestBase64.java
> b/test/jdk/java/util/Base64/TestBase64.java
> --- a/test/jdk/java/util/Base64/TestBase64.java
> +++ b/test/jdk/java/util/Base64/TestBase64.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -24,7 +24,7 @@
> /**
> * @test
> * @bug 4235519 8004212 8005394 8007298 8006295 8006315 8006530 8007379
> 8008925
> - * 8014217 8025003 8026330 8028397 8129544 8165243 8176379
> + * 8014217 8025003 8026330 8028397 8129544 8165243 8176379 8222187
> * @summary tests java.util.Base64
> * @library /test/lib
> * @build jdk.test.lib.RandomFactory
> @@ -144,6 +144,10 @@
> testDecoderKeepsAbstinence(Base64.getDecoder());
> testDecoderKeepsAbstinence(Base64.getUrlDecoder());
> testDecoderKeepsAbstinence(Base64.getMimeDecoder());
> +
> +// tests patch addressing JDK-8222187
> +// https://bugs.openjdk.java.net/browse/JDK-8222187
> +testJDK_8222187();
> }
>
> private static void test(Base64.Encoder enc, Base64.Decoder dec,
> @@ -607,4 +611,26 @@
> }
> }
> }
> +
> +private static void testJDK_8222187() throws Throwable {
> +byte[] orig = "12345678".getBytes("US-ASCII");
> +byte[] encoded = Base64.getEncoder().encode(orig);
> +// decode using different buffer sizes, up to a longer one than
> needed
> +for (int bufferSize = 1; bufferSize <= encoded.length + 1;
> bufferSize++) {
> +try (
> +InputStream in = Base64.getDecoder().wrap(
> +new ByteArrayInputStream(encoded));
> +ByteArrayOutputStream baos = new ByteArrayOutputStream();
> +) {
> +byte[] buffer = new byte[bufferSize];
> +int read;
> +while ((read = in.read(buffer, 0, bufferSize)) >= 0) {
> +baos.write(buffer, 0, read);
> +}
> +// compare result, output info if lengths do not match
> +byte[] decoded = baos.toByteArray();
> +checkEqual(decoded, orig, "Base64 stream decoding failed!");
> +}
> +}
> +}
> }
>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: 8249774: Add java/foreign/TestMismatch.java to ProblemList.txt
+1 > On Jul 20, 2020, at 3:25 PM, Daniel Fuchs wrote: > > Hi, > > Please find below a fix for: > > 8249774: Add java/foreign/TestMismatch.java to ProblemList.txt > https://bugs.openjdk.java.net/browse/JDK-8249774 > > The test has failed twice in timeout on macOS in the tier1 CI. > Requesting to put it into a problem list until a fix is proposed. > > > best regards, > > -- daniel > > patch: > > > > diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt > --- a/test/jdk/ProblemList.txt > +++ b/test/jdk/ProblemList.txt > @@ -559,6 +559,12 @@ > > > > +# jdk_foreign > + > +java/foreign/TestMismatch.java 8249684 macosx-all > + > + > + > # jdk_lang > > java/lang/StringCoding/CheckEncodings.sh 7008363 generic-all Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: JDK-8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the end
Hi Roger, I will plan to push tomorrow morning pending any last minute reviews Best Lance > On Jul 21, 2020, at 9:57 AM, Roger Riggs wrote: > > Hi Rafaello, Lance, > > That looks good to me. > > Thanks, Roger > > On 7/19/20 2:31 PM, Lance Andersen wrote: >> Hi Raffaello, >> >> I think the changes look reasonable. >> >> I have created a webrev, >> http://cr.openjdk.java.net/~lancea/8222187/webrev.00/ >> <http://cr.openjdk.java.net/~lancea/8222187/webrev.00/>, for others to >> review as it is a bit easier than the patch. >> >> I have also run the JCK tests in this area as well as mach5 tiers1-3 (which >> I believe Roger has also) >> >> Best >> Lance >> >>> On Jul 15, 2020, at 5:44 PM, Raffaello Giulietti >>> mailto:raffaello.giulie...@gmail.com>> >>> wrote: >>> >>> Hi Roger, >>> >>> On 2020-07-15 22:21, Roger Riggs wrote: >>>> Hi Raffaello, >>>> Base64.java: >>>> 2: Please update 2nd copyright year to 2020 >>> >>> I was unsure what to do here, thanks for guidance. >>> I also removed the seemingly useless import at former L33. >>> >>> >>> >>>> leftovers(): >>>> 996: "&" the shortcutting && is more typical in a condition. >>>> 997: the -= is buried in an expression and a reader might miss it. >>> >>> Corrected, as well as the analogous -= usage for wpos now at L1106-7 >>> >>> >>>> 1053: "can be reallocated to other vars": not a useful comment, >>>> reflecting on only one possible implementation that is out of the control >>>> of the developer. >>>> I'd almost rather see 'len' than 'limit - off'; it might be informative to >>>> the reader if 'limit' was declared final. (1056:) >>> >>> Done, as well as declaring i and v final in the loop. >>> >>> >>> >>>> TestBase54.java: >>>> 2: Update 2nd copyright year to 2020 >>>> 27: Please add the bug number to the @bug line. >>>> Style-wise, I would remove the blank lines at the beginning of blocks. >>>> (1095, 1115) >>> >>> Done. >>> >>> >>>> Thanks, Roger >>>> On 7/14/20 11:47 AM, Raffaello Giulietti wrote: >>>>> Hi Roger, >>>>> >>>>> here's the latest version of the patch. >>>>> >>>>> I did: >>>>> * Withdraw the simplification in encodedOutLength(), as it is indeed out >>>>> of scope for this bug fix. >>>>> * Restore field names in DecInputStream except for nextin (now wpos) and >>>>> nextout (now rpos) because they have slightly different semantics. That's >>>>> just for clarity. I would have nothing against reusing the old names for >>>>> the proposed purpose. >>>>> * Switch back to the original no-arg read() implementation. >>>>> * Adjust comment continuation lines. >>>>> * Preserve the proposed logic of read(byte[], int, int) and the >>>>> supporting methods. >>>>> >>>>> Others needed changes are: >>>>> * Correct the copyright years: that's something better left to Oracle. >>>>> * Remove the apparently useless import at L33. I could build and run >>>>> without it. >>>>> >>>>> >>>>> Greetings >>>>> Raffaello >>>>> >>> >>> >>> >>> >>> # HG changeset patch >>> # User lello >>> # Date 1594848775 -7200 >>> # Wed Jul 15 23:32:55 2020 +0200 >>> # Node ID f7407f35e83ab508f0e6deab4f776bb1a1c85e68 >>> # Parent a5649ebf94193770ca763391dd63807059d333b4 >>> 8222187: java.util.Base64.Decoder stream adds unexpected null bytes at the >>> end >>> Reviewed-by: TBD >>> Contributed-by: Raffaello Giulietti >> <mailto:raffaello.giulie...@gmail.com>> >>> >>> diff --git a/src/java.base/share/classes/java/util/Base64.java >>> b/src/java.base/share/classes/java/util/Base64.java >>> --- a/src/java.base/share/classes/java/util/Base64.java >>> +++ b/src/java.base/share/classes/java/util/Base64.java >>> @@ -1,5 +1,5 @@ >>> /* >>> - * Copyright (c) 2012, 2019, Oracle and/o
Re: JDK 16 RFR of JDK-8250221: Address use of default constructors in java.logging
+1 > On Jul 23, 2020, at 1:37 PM, Joe Darcy wrote: > > ErrorManager.jav <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR(M): 8249963: Make the zlib implementation selectively configurable at startup
undled zlib compatible with the > system or the other zlib implementations which all do not have this > change. Notice that if we're building with the system zlib, we are > already using this setting today because in that case "zconf.h" is > taken from the system include path. > > Finally, it has to be noticed that although we are loading libzip > early there are still two call sites of zlib functionality which won't > be able to benefit from the new implementations because they either > statically link in parts of the bundled zlib version or directly and > dynamically link against the system zlib version. That's > libsplashscreen and libjli. Both are only used at startup and both > only make limited use of zlib (if they use it at all) so I don't think > that they are relevant. > > Risks > - > > Unfortunately ZipInputStream/ZipOutputStream have been designed such > that it is very easy to use them in an unsupported way. It is in > general not possible to read a ZipEntry from a ZipInputStream and > write that exact ZipEntry into a ZipOutputStream followed by the > inflated and re-deflated data of that entry. However this naive > approach is sometimes used to copy zip entries from a zip input file > into another zip output file. This approach will only work in the case > where the exact same deflate implementation and compression was used > for the initial compression like for the recompression. > > This is because the ZipEntry will contain the compressed size of the > data belonging to that entry. But the Deflate format doesn't mandate a > specific, fixed compression algorithm which will always result in the > same compressed size. Instead, different implementations are free to > use different approaches for compression which can result in valid > entries but potentially with a different compressed size. > > Actually, even if just a single implementation is used for both > compression and decompression, it is still not possible to detect the > compression level from the compressed data. It is therefore possible > that decompressing and re-compressing that data into a ZipOutputStream > will result in a different compressed size. However, ZipOutputStream > will throw an exception if the compressed size entry in a ZipEntry is > different from the actual size of the compressed data. > > I've recently fixed two such cases in the OpenJDK (JDK-8240333 [14], > JDK-8240235 [15]) itself, but there may be other such use cases in the > wild which may throw an "ZipException: invalid entry compressed size > (expected 66 but got 67 bytes)". If that happens, the user can either > fix his code (which is trivial and advised because of the problems > explained before) or simply remove the new system properties in order > to fall back to the bundled or system implementation. > > [1] https://www.zlib.net/ > [2] https://github.com/cloudflare/zlib > [3] https://chromium.googlesource.com/chromium/src/third_party/zlib/ > [4] https://github.com/simonis/zlib-bench/blob/master/Results-ojdk.md > [5] https://github.com/simonis/zlib-bench/blob/master/Results.md > [6] https://github.com/simonis/zlib-bench/tree/master/lib > [7] https://github.com/madler/zlib > [8] https://chromium.googlesource.com/chromium/src/third_party/zlib/ > [9] https://github.com/zlib-ng/zlib-ng > [10] https://software.intel.com/en-us/articles/intel-ipp-zlib-coding-functions > [11] https://github.com/cloudflare/zlib > [12] https://github.com/intel/isa-l > [13] https://bugs.openjdk.java.net/browse/JDK-8237750 > [14] https://bugs.openjdk.java.net/browse/JDK-8240333 > [15] https://bugs.openjdk.java.net/browse/JDK-8240235 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
+1
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Sent from my iPhone
> On Jul 23, 2020, at 6:04 PM, Joe Darcy wrote:
>
> Hello,
>
> Martin, how would you prefer these changes, or equivalent ones, to get into
> the java.util.concurrent upstream sources?
>
> Several classes in java.util.concurrent rely on default constructors, which
> is not recommended. Please review the patch below which removes them along
> with the corresponding CSR
> (https://bugs.openjdk.java.net/browse/JDK-8250241); webrev at
> http://cr.openjdk.java.net/~darcy/8250240.0/.
>
> Thanks,
>
> -Joe
>
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -77,6 +77,11 @@
> public abstract class AbstractExecutorService implements ExecutorService {
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public AbstractExecutorService() {}
> +
> +/**
> * Returns a {@code RunnableFuture} for the given runnable and default
> * value.
> *
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> --- a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java Thu
> Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java Thu
> Jul 23 15:01:27 2020 -0700
> @@ -242,6 +242,11 @@
> private static final int SIGNAL = 1 << 16; // true if joiner waiting
> private static final int SMASK= 0x; // short bits for tags
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public ForkJoinTask() {}
> +
> static boolean isExceptionalStatus(int s) { // needed by subclasses
> return (s & THROWN) != 0;
> }
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> --- a/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -166,6 +166,11 @@
> private static final long serialVersionUID = 5232453952276485070L;
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveAction() {}
> +
> +/**
> * The main computation performed by this task.
> */
> protected abstract void compute();
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> --- a/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java Thu
> Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java Thu
> Jul 23 15:01:27 2020 -0700
> @@ -69,6 +69,11 @@
> private static final long serialVersionUID = 5232453952276485270L;
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveTask() {}
> +
> +/**
> * The result of the computation.
> */
> @SuppressWarnings("serial") // Conditionally serializable
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -65,6 +65,11 @@
>
> private static final long serialVersionUID = 7373984972572414692L;
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public AbstractQueuedLongSynchronizer() {}
> +
> /*
> * To keep sources in sync, the remainder of this source file is
> * exactly cloned from AbstractQueuedSynchronizer, replacing class
>
Re: JDK 16 RFR of 8250580: Address reliance on default constructors in java.rmi
+1
> On Jul 24, 2020, at 10:45 PM, Joe Darcy wrote:
>
> Hello,
>
> Another bug in the quest to remove use of default constructors in the JDK's
> public API, this time in the java.rmi module:
>
> webrev: http://cr.openjdk.java.nhet/~darcy/8250580.0/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8250581
>
> Patch below; thanks,
>
> -Joe
>
> --- old/src/java.rmi/share/classes/java/rmi/server/RMIClassLoaderSpi.java
> 2020-07-24 19:42:16.353847343 -0700
> +++ new/src/java.rmi/share/classes/java/rmi/server/RMIClassLoaderSpi.java
> 2020-07-24 19:42:15.645847343 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -62,6 +62,11 @@
> public abstract class RMIClassLoaderSpi {
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RMIClassLoaderSpi() {}
> +
> +/**
> * Provides the implementation for
> * {@link RMIClassLoader#loadClass(URL,String)},
> * {@link RMIClassLoader#loadClass(String,String)}, and
>
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: JDK 16 RFR of JDK-8250578: Address reliance on default constructors in javax.sql
+1
> On Jul 24, 2020, at 10:25 PM, Joe Darcy wrote:
>
> Hello,
>
> Another module, another use of a default constructor. Please review the fix
> and CSR for:
>
> JDK-8250578: Address reliance on default constructors in javax.sql
> CSR: https://bugs.openjdk.java.net/browse/JDK-8250579
> http://cr.openjdk.java.net/~darcy/8250578.0/
>
> Patch below.
>
> Thanks,
>
> -Joe
>
> ---
> old/src/java.sql.rowset/share/classes/javax/sql/rowset/RowSetMetaDataImpl.java
> 2020-07-24 19:16:43.633847343 -0700
> +++
> new/src/java.sql.rowset/share/classes/javax/sql/rowset/RowSetMetaDataImpl.java
> 2020-07-24 19:16:42.941847343 -0700
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -52,6 +52,10 @@
> * @since 1.5
> */
> public class RowSetMetaDataImpl implements RowSetMetaData, Serializable {
> +/**
> + * Constructs a {@code RowSetMetaDataImpl} object.
> + */
> +public RowSetMetaDataImpl() {}
>
> /**
> * The number of columns in the RowSet object that created
>
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: RFR [16/java.xml] 8249643: Clarify DOM documentation
Hi Joe, The changes look fine. Best Lance > On Jul 28, 2020, at 1:25 PM, Joe Wang wrote: > > Hello, > > Please review a doc clarification for the org.w3c.dom package. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8249643 > CSR: https://bugs.openjdk.java.net/browse/JDK-8249904 > > webrev: http://cr.openjdk.java.net/~joehw/jdk16/8249643/webrev_02/ > > Thanks, > Joe > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Re: RFR [16/java.xml] 8250638: Address reliance on default constructors in java.xml
+1
> On Jul 30, 2020, at 12:20 PM, Joe Wang wrote:
>
> Hello,
>
> Please review a change to remove reliance of default constructors in java.xml.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8250638
> CSR: https://bugs.openjdk.java.net/browse/JDK-8250800
>
> Patch:
>
> diff --git a/src/java.xml/share/classes/org/xml/sax/HandlerBase.java
> b/src/java.xml/share/classes/org/xml/sax/HandlerBase.java
> --- a/src/java.xml/share/classes/org/xml/sax/HandlerBase.java
> +++ b/src/java.xml/share/classes/org/xml/sax/HandlerBase.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -58,7 +58,10 @@
> public class HandlerBase
> implements EntityResolver, DTDHandler, DocumentHandler, ErrorHandler
> {
> -
> +/**
> + * Constructs a {@code HandlerBase}.
> + */
> +public HandlerBase() {}
>
>
> // Default implementation of the EntityResolver interface.
> diff --git
> a/src/java.xml/share/classes/org/xml/sax/helpers/DefaultHandler.java
> b/src/java.xml/share/classes/org/xml/sax/helpers/DefaultHandler.java
> --- a/src/java.xml/share/classes/org/xml/sax/helpers/DefaultHandler.java
> +++ b/src/java.xml/share/classes/org/xml/sax/helpers/DefaultHandler.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights
> reserved.
> + * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights
> reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> @@ -70,7 +70,10 @@
> public class DefaultHandler
> implements EntityResolver, DTDHandler, ContentHandler, ErrorHandler
> {
> -
> +/**
> + * Constructs a {@code DefaultHandler}.
> + */
> +public DefaultHandler() {}
>
> ////
> // Default implementation of the EntityResolver interface.
>
> Thanks,
> Joe
>
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
RFR 8251202: Add missing javadoc comments
Hi all, Please review the webrev for: https://bugs.openjdk.java.net/browse/JDK-8251202 <https://bugs.openjdk.java.net/browse/JDK-8251202>, which adds missing javadoc comments to java.util.zip.ZipConstants. The webrev can be found at: http://cr.openjdk.java.net/~lancea/8251205/webrev.00/ The CSR, which has been approved, can be found at: https://bugs.openjdk.java.net/browse/JDK-8251206 <https://bugs.openjdk.java.net/browse/JDK-8251206> Best Lance ------ Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR 8251202: Add missing javadoc comments
Hi Naoto Thank you for catching that Will add them before I push the change Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPhone > On Aug 6, 2020, at 7:35 PM, naoto.s...@oracle.com wrote: > > Looks good, Lance. > > nit: a couple of comments (line 128,133) don't end with periods. > > Naoto > >> On 8/6/20 3:32 PM, Lance Andersen wrote: >> Hi all, >> Please review the webrev for: >> https://bugs.openjdk.java.net/browse/JDK-8251202 >> <https://bugs.openjdk.java.net/browse/JDK-8251202>, which adds missing >> javadoc comments to java.util.zip.ZipConstants. >> The webrev can be found at: >> http://cr.openjdk.java.net/~lancea/8251205/webrev.00/ >> The CSR, which has been approved, can be found at: >> https://bugs.openjdk.java.net/browse/JDK-8251206 >> <https://bugs.openjdk.java.net/browse/JDK-8251206> >> Best >> Lance >> -- >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com
Re: 8251160: Fix "no comment" warnings in java.logging
Hi Daniel, The changes look good. I added myself as a reviewer to the CSR Best Lance > On Aug 13, 2020, at 8:00 AM, Daniel Fuchs wrote: > > Hi, > > Please find below a doc-only fix for: > > 8251160: Fix "no comment" warnings in java.logging > https://bugs.openjdk.java.net/browse/JDK-8251160 > > CSR: https://bugs.openjdk.java.net/browse/JDK-8251534 > > webrev: > http://cr.openjdk.java.net/~dfuchs/webrev_8251160/webrev.00/ > > The documentation of the serial form of java.util.logging.Level > and java.util.logging.LogRecord is improved to avoid doclint > warnings. > > best regards, > > -- daniel Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
RFR 8251208: Fix javadoc warnings in java.sql and java.sql.rowsets
Hi all, Please review the fix to address javadoc warnings in java.sql and java.sql.rowsets The webrev can be found at: http://cr.openjdk.java.net/~lancea/8251208/webrev.00/ And the CSR at: https://bugs.openjdk.java.net/browse/JDK-8251834 Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR 8251208: Fix javadoc warnings in java.sql and java.sql.rowsets
Hi Joe,
Thank you for the review.
Changes had been made to the typos below (went back to far in IntelliJ fixing
another issue :-( )
CSR has also been updated to fix the typos (missed I guess due to lack of
coffee this am ;-) )
http://cr.openjdk.java.net/~lancea/8251208/webrev.01/index.html is the updated
webrev
Best
Lance
> On Aug 14, 2020, at 1:20 PM, Joe Wang wrote:
>
> Hi Lance,
>
> Looks good to me overall.
>
> Minor typos in the CSR:
> Address the Fix "no comment" warnings in java.sql and
> java.sql.rowsetsgenerated by javadoc -Xdoclint
> ^ remove Fix ^
> missing a space between rowsetsgenerated
>
> java.sql and java.sql.rowset contain several iclasses
> classes
>
> Compatibility Risk: the last word "clas" -> class
>
> Webrev:
> Need to update copyright year for classes: SerialRef.java, SerialStruct.java,
> SQLClientInfoException.java
>
> StatementEvent.java: the 1st statement (line 47) was supposed for the 2nd
> field (line55), while the 2nd (line 52) for the 1st (line 49) :-)
> e.g.:
> 46 /**
> 47 * The {@code PreparedStatement} that is being closed or is
> invalid.
> 48 */
> 49 private SQLExceptionexception;
> 50
> 51 /**
> 52 * The {@code SQLException} the driver is about to throw to the
> application.
> 53 */
> 54 @SuppressWarnings("serial") // Not statically typed as
> Serializable
> 55 private PreparedStatement statement;
>
>
> -Joe
>
> On 8/14/2020 3:37 AM, Lance Andersen wrote:
>> Hi all,
>>
>> Please review the fix to address javadoc warnings in java.sql and
>> java.sql.rowsets
>>
>> The webrev can be found at:
>> http://cr.openjdk.java.net/~lancea/8251208/webrev.00/
>> <http://cr.openjdk.java.net/~lancea/8251208/webrev.00/>
>>
>> And the CSR at: https://bugs.openjdk.java.net/browse/JDK-8251834
>> <https://bugs.openjdk.java.net/browse/JDK-8251834>
>>
>> Best
>> Lance
>> --
>>
>>
>>
>>
>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
>>
>>
>>
>>
>
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
RFR 8252128: Remove javax.transaction Exception references
HI all, The following patch removes some extraneous references to javax.transaction exceptions and removes a couple of unused fields that represented serialized versions of the exceptions. This was missed in the removal of the CORBA and Java EE modules. The webrev can be found at: http://cr.openjdk.java.net/~lancea/8252128/webrev.00/ Mach5 jdk-tier1, jdk-tier2, jdk-tier3 are clean (as expected) Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR 8251203: Fix "no comment" warnings in java.base/java.lang and java/io
Hi Roger, Looks good overall. Friendly reminder to update the copyright in the files such as File.java, UncheckedIOException.java, AbstractStringBuilder.java etc... > On Aug 21, 2020, at 11:09 AM, Roger Riggs wrote: > > Please review the addition of comments to classes and fields to resolve > javadoc "no comment" warnings in the java.lang and java.io packages. > The comments are derived from the existing behavior and context. > > Webrev: >http://cr.openjdk.java.net/~rriggs/webrev-nocomment-8251203-1/ > > CSR: > https://bugs.openjdk.java.net/browse/JDK-8252129 > > Issue: > https://bugs.openjdk.java.net/browse/JDK-8251203 > > Thanks, Roger > > Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package
Hi Joe, This looks OK. > On Aug 21, 2020, at 2:23 PM, Joe Wang wrote: > > Pelase review a patch to add the missing @return, @throws, @param statements > in the java.xml package (excluding the DOM component). > > JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 > CSR: https://bugs.openjdk.java.net/browse/JDK-8251995 > > webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_01/ > > Thanks, > Joe Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package
Looks OK Joe > On Aug 24, 2020, at 5:44 PM, Joe Wang wrote: > > Hi all, adding Roger's comment for the make file to webrev_02 (the only > change to webrev_01 is Docs.gmk): > > http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_02/ > > Thanks, > Joe > > On 8/21/20 12:49 PM, naoto.s...@oracle.com wrote: >> +1 >> >> Naoto >> >> On 8/21/20 12:24 PM, Lance Andersen wrote: >>> Hi Joe, >>> >>> This looks OK. >>> >>> >>> >>>> On Aug 21, 2020, at 2:23 PM, Joe Wang wrote: >>>> >>>> Pelase review a patch to add the missing @return, @throws, @param >>>> statements in the java.xml package (excluding the DOM component). >>>> >>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 >>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8251995 >>>> >>>> webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_01/ >>>> >>>> Thanks, >>>> Joe >>> >>> >>> Best >>> Lance >>> -- >>> >>> >>> >>> >>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >>> Oracle Java Engineering >>> 1 Network Drive >>> Burlington, MA 01803 >>> lance.ander...@oracle.com >>> >>> >>> >>> > Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: [16] RFR: 8251182: Fix "no comment" warnings in java.naming
Hi Aleksei The changes look good > On Aug 25, 2020, at 11:03 AM, Aleks Efimov wrote: > > Hi, > > The documentation of classes from "java.naming" module needs to be improved > to resolve javadoc -Xdoclint "no comment" warnings. > > JBS issue: https://bugs.openjdk.java.net/browse/JDK-8251182 > Webrev: http://cr.openjdk.java.net/~aefimov/8251182/00/index.html > CSR: https://bugs.openjdk.java.net/browse/JDK-8252310 > > With Best Regards, > Aleksei Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: [16] RFR: 8251182: Fix "no comment" warnings in java.naming
+1 > On Aug 26, 2020, at 7:46 AM, Aleks Efimov wrote: > > Hi Roger, Lance, Daniel, > > Thanks for your reviews and comments! > New webrev with suggested modifications can be viewed at this location: > http://cr.openjdk.java.net/~aefimov/8251182/01 > > The list of modifications: > - Added @java.io.Serial annotation to 'serialVersionUID' declarations > - Removed trailing periods from @param/@throws > - Added generic first line comments to writeObject/readObject methods > > The new version of patch and webrev files have been uploaded to the CSR issue. > > Kind Regards, > Aleksei > > On 25/08/2020 20:23, Daniel Fuchs wrote: >> Hi Aleksei, >> >> LGTM. >> >> You could also add the @Seial annotation to serialVersionUID >> declarations. >> >> best regards, >> >> -- daniel >> >> On 25/08/2020 16:03, Aleks Efimov wrote: >>> Hi, >>> >>> The documentation of classes from "java.naming" module needs to be improved >>> to resolve javadoc -Xdoclint "no comment" warnings. >>> >>> JBS issue: https://bugs.openjdk.java.net/browse/JDK-8251182 >>> Webrev: http://cr.openjdk.java.net/~aefimov/8251182/00/index.html >>> CSR: https://bugs.openjdk.java.net/browse/JDK-8252310 >>> >>> With Best Regards, >>> Aleksei >> > Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR 8252265: Replace @exception with @throws java.util.logging package
This looks ok overall. Not specific to your change but … I did find it strange that FileHandler had a few methods which listed IllegalArgumentException multiple times (vs. including the possible scenarios for the exception as part of 1 @throws) as I have not noticed that elsewhere in the JDK but perhaps I have missed it. > On Aug 26, 2020, at 12:38 PM, Vipin Sharma wrote: > > Hi, > > Please review and sponsor the fix for replacing @exception with @throws in > java.util.logging package. > > Issue: https://bugs.openjdk.java.net/browse/JDK-8252265 > Webrev: https://cr.openjdk.java.net/~vsharma/8252265/ > > It came up in the below discussion, starting this cleanup with > java.util.logging package. > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068172.html > > Regards, > Vipin Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8250968: Symlinks attributes not preserved when using jarsigner on zip files
Hi Sean,
I think the changes are OK in the latest version. A couple of the files have a
2019 copyright still.
> On Aug 27, 2020, at 10:58 AM, Weijun Wang wrote:
>
>
>
>> On Aug 27, 2020, at 10:36 AM, Seán Coffey wrote:
>>
>> Thanks for the review Max. Comments inline..
>>
>> On 27/08/2020 14:45, Weijun Wang wrote:
>>> I’m OK with using one warning, but prefer it to a little more formal like
>>> "POSIX file permission and/or symlink attributes detected…”.
>>>
>>> One nit in ZipFile.java:
>>>
>>> 1098 // only set posix perms value via ZipEntry constructor
>>> for now
>>> 1099 @Override
>>> 1100 public int getExtraAttributes(ZipEntry ze) {
>>>
>>> Maybe you can just remove the comment.
>>>
>>> Do you also want to rename the “posixPermsDetected" field and loacl
>>> variable “perms” in JarSigner.java?
>>
>> Good points. Edits made.
>>
>> http://cr.openjdk.java.net/~coffeys/webrev.8250968.v3/webrev/
>>
>>>
>>> I’m not sure about the test but if zipfs is able to keep permissions inside
>>> a zip file then that POSIX byte (or whatever it’s named) is already there
>>> and we can modify it to include more bits.
>>
>> Looks like it was a conscious design decision to only allow recording of
>> POSIX permission bits for this field (& 0xFFF). I don't see anything about
>> symlink support in zipfs docs.
>
> As long as that *byte* is there and it’s not difficult to locate, we can
> manually add the *bit* for symlink and see if jarsigner can keep it.
We can create an RFE for adding support for this with Zip FS.
>
> —Max
>
>>
>> regards,
>> Sean.
>>
>>>
>>> No other comment.
>>>
>>> Thanks,
>>> Max
>>>
>>>
>>>> On Aug 27, 2020, at 3:26 AM, Seán Coffey wrote:
>>>>
>>>> updated webrev:
>>>> http://cr.openjdk.java.net/~coffeys/webrev.8250968.v2/webrev/
>>>>
>>>> regards,
>>>> Sean.
>>>>
>>>> On 27/08/2020 07:42, Seán Coffey wrote:
>>>>> Hi Max,
>>>>>
>>>>> I looked at updating the warning string but figured that it might have
>>>>> been of no interest to end users. How about this edit then ?
>>>>>
>>>>> +{"posix.attributes.detected", "POSIX file permission attributes
>>>>> detected. These attributes are ignored when signing and are not protected
>>>>> by the signature."},
>>>>>
>>>>>>> replace with:
>>>>> +{"extra.attributes.detected", "POSIX file permission/symlink
>>>>> attributes detected. These attributes are ignored when signing and are
>>>>> not protected by the signature."},
>>>>>
>>>>> regards,
>>>>> Sean.
>>>>>
>>>>> On 26/08/2020 23:15, Weijun Wang wrote:
>>>>>> Are you going to update the warning text or create a new one?
>>>>>>
>>>>>> Thanks,
>>>>>> Max
>>>>>>
>>>>>>> On Aug 26, 2020, at 2:26 PM, Seán Coffey wrote:
>>>>>>>
>>>>>>> This is a follow on from the recent 8218021 fix. The jarsigner tool
>>>>>>> removes symlink attribute data from zipfiles when signing them.
>>>>>>> jarsigner should preserve this data. The fix involves preserving the 16
>>>>>>> bits associated with the file attributes (instead of the current 12).
>>>>>>> That's done in ZipFile. All other changes are just a refactor of the
>>>>>>> variable name.
>>>>>>>
>>>>>>> I haven't been able to automate a test for this since zipfs doesn't
>>>>>>> seem to support symlinks. Manual testing looks good.
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8250968
>>>>>>> http://hmsjpse.uk.oracle.com/home/scoffey/ws/jdk-jdk/open/webrev.8250968/webrev/index.html
>>>>>>>
>>>>>>> regards,
>>>>>>> Sean.
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: RFR: 8252552: DecimalFormat javadoc contains HTML tags in example code
Hi Naoto,
Looks OK to me.
Best
Lance
> On Aug 31, 2020, at 12:22 PM, Naoto Sato wrote:
>
> Hello,
>
> Please review this simple javadoc fix to:
>
> https://bugs.openjdk.java.net/browse/JDK-8252552
>
> The proposed diff is inserted here:
>
> ---
> --- old/src/java.base/share/classes/java/text/DecimalFormat.java 2020-08-31
> 09:07:22.0 -0700
> +++ new/src/java.base/share/classes/java/text/DecimalFormat.java 2020-08-31
> 09:07:22.0 -0700
> @@ -338,9 +338,9 @@
> *
> * Example
> *
> - * {@code
> - * // Print out a number using the localized number, integer,
> currency,
> - * // and percent format for each locale
> + * {@code
> + * // Print out a number using the localized number, integer, currency,
> + * // and percent format for each locale}{@code
> * Locale[] locales = NumberFormat.getAvailableLocales();
> * double myNumber = -1234.56;
> * NumberFormat form;
> ---
>
> It is simply to take those HTML tags out of {@code} snippet.
>
> Naoto
Best
Lance
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: RFR [16/java.xml] 8252354 : Properties :: storeToXML method does not throw ClassCastException when supplied non strings.
Hi Joe Looks fine to me -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com > On Sep 1, 2020, at 5:12 PM, Joe Wang wrote: > > Please review a fix to report CCE when keys or values are not Strings as > specified. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8252354 > > webrev: http://cr.openjdk.java.net/~joehw/jdk16/8252354/webrev/ > > Thanks, > Joe
Re: RFR 8252537: Replace @exception with @throws for core-libs
Hi Vipin I can sponsor once the review is complete as a vast majority are in java.sql I will look at this tomorrow > On Sep 3, 2020, at 3:33 PM, Vipin Sharma wrote: > > Hi, > > Please review and sponsor the fix for replacing @exception with @throws for > core-libs. > > Issue: https://bugs.openjdk.java.net/browse/JDK-8252537 > Webrev: https://cr.openjdk.java.net/~vsharma/8252537 > > As suggested in the previous discussion > <https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-September/068486.html>, > this webrev has a consolidated fix for all subtasks of the JDK-8252536 > <https://bugs.openjdk.java.net/browse/JDK-8252536>. > There is no change in the indentation as part of this webrev. > > Regards, > Vipin Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8250968: Symlinks attributes not preserved when using jarsigner on zip files
On Mon, 7 Sep 2020 13:48:57 GMT, Sean Coffey wrote: > Continuation of RFR thread from > http://mail.openjdk.java.net/pipermail/security-dev/2020-August/022373.html > > CSR has been approved. I think this looks good over all Sean. In your SymLinkTest, I probably would have the test delete the file if it exists prior to writing to it. - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/56
Re: RFR: 8250968: Symlinks attributes not preserved when using jarsigner on zip files [v2]
On Mon, 7 Sep 2020 18:57:11 GMT, Sean Coffey wrote: >> Continuation of RFR thread from >> http://mail.openjdk.java.net/pipermail/security-dev/2020-August/022373.html >> >> CSR has been approved. > > Sean Coffey has updated the pull request incrementally with one additional > commit since the last revision: > > Copyright and test clean up Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/56
Re: RFR: 8252830: Correct missing javadoc comments in java.rmi module [v2]
On Tue, 8 Sep 2020 18:55:40 GMT, Roger Riggs wrote: >> 8252830: Correct missing javadoc comments in java.rmi module > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Added @java.io.Serial annotation to serializable methods and fields Hi Roger, The combined changes to both commits look good - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/79
Re: RFR: 8252830: Correct missing javadoc comments in java.rmi module [v3]
On Tue, 8 Sep 2020 19:44:21 GMT, Roger Riggs wrote: >> 8252830: Correct missing javadoc comments in java.rmi module > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Added java.io.Serial to java.rmi.activation.ActivationID Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/79
Re: RFR: 8157729: examples in LinkedHashMap and LinkedHashSet class doc use raw types
On Tue, 8 Sep 2020 22:32:47 GMT, Stuart Marks wrote:
> Add some generics and wrap in `{@code}` to protect angle brackets.
Looks good
-
Marked as reviewed by lancea (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/87
Re: RFR: 8157729: examples in LinkedHashMap and LinkedHashSet class doc use raw types [v2]
On Tue, 8 Sep 2020 23:00:21 GMT, Stuart Marks wrote:
>> Add some generics and wrap in `{@code}` to protect angle brackets.
>
> Stuart Marks has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update copyright years.
Marked as reviewed by lancea (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/87
Re: RFR: 8251495: Clarify DOM documentation
On Wed, 9 Sep 2020 22:56:14 GMT, Joe Wang wrote: > Revert changes made by JDK-8249643, removing the implNote. Based on the spec review and follow-on discussions, this change makes sense - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/100
Re: RFR: 8253066: typo in Stream.mapMulti
On Fri, 11 Sep 2020 22:20:48 GMT, Stuart Marks wrote: > A simple typo fix. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/137
Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance)
On Fri, 11 Sep 2020 14:17:45 GMT, Jaikiran Pai wrote: > Can I please get a review and a sponsor for this patch which fixes the issue > reported in > https://bugs.openjdk.java.net/browse/JDK-8244706? > The commit here sets the `OS` header flag to `255` (which represents > `unknown`) as noted in [1]. A new test has been > included in this commit to verify the change. Furthermore, this doesn't > impact the `java.util.zip.GZIPInputStream` > since it ignores [2] this header value while reading the headers from the > stream. [1] > https://tools.ietf.org/html/rfc1952#page-7 [2] > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173 Hi Jaikiran, The change seems fine an inline with the RFC. I can sponsor this once we have another review. I have run the JCK tests for Zip/Gzip/Jar and Mach5 JDK tier1, tier2 and tier3 - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/130
Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null
On Mon, 14 Sep 2020 22:18:34 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Hi Naoto, The change looks fine. I might have created a CSR just to track the doc change. - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/159
Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance)
Hi Joe, I guess it could. Given it is not used within the implementation(or defined outside of the spec), I will defer to you preference :-) > On Sep 14, 2020, at 6:49 PM, Joe Darcy wrote: > > Should issue have a CSR review for the behavior change? > > -Joe > > On 9/12/2020 7:25 PM, Jaikiran Pai wrote: >> On Sat, 12 Sep 2020 17:38:34 GMT, Lance Andersen wrote: >> >>>> Can I please get a review and a sponsor for this patch which fixes the >>>> issue reported in >>>> https://bugs.openjdk.java.net/browse/JDK-8244706? >>>> The commit here sets the `OS` header flag to `255` (which represents >>>> `unknown`) as noted in [1]. A new test has been >>>> included in this commit to verify the change. Furthermore, this doesn't >>>> impact the `java.util.zip.GZIPInputStream` >>>> since it ignores [2] this header value while reading the headers from the >>>> stream. [1] >>>> https://tools.ietf.org/html/rfc1952#page-7 [2] >>>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173 >>> Hi Jaikiran, >>> >>> The change seems fine an inline with the RFC. I can sponsor this once we >>> have another review. >>> >>> I have run the JCK tests for Zip/Gzip/Jar and Mach5 JDK tier1, tier2 and >>> tier3 >> Thank you Lance for the review and running the tests. I'll wait for another >> review before initiating the integrate >> command. >> >> - >> >> PR: https://git.openjdk.java.net/jdk/pull/130 Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance)
Hi Joe, Ok, will create one tomorrow. Best Lance > On Sep 14, 2020, at 7:07 PM, Joe Darcy wrote: > > Hi Lance, > > I'd prefer to err on the side of having a CSR; thanks, > > -Joe > > On 9/14/2020 3:55 PM, Lance Andersen wrote: >> Hi Joe, >> >> I guess it could. Given it is not used within the implementation(or defined >> outside of the spec), I will defer to you preference :-) >> >>> On Sep 14, 2020, at 6:49 PM, Joe Darcy >> <mailto:joe.da...@oracle.com>> wrote: >>> >>> Should issue have a CSR review for the behavior change? >>> >>> -Joe >>> >>> On 9/12/2020 7:25 PM, Jaikiran Pai wrote: >>>> On Sat, 12 Sep 2020 17:38:34 GMT, Lance Andersen >>> <mailto:lan...@openjdk.org>> wrote: >>>> >>>>>> Can I please get a review and a sponsor for this patch which fixes the >>>>>> issue reported in >>>>>> https://bugs.openjdk.java.net/browse/JDK-8244706? >>>>>> <https://bugs.openjdk.java.net/browse/JDK-8244706?> >>>>>> The commit here sets the `OS` header flag to `255` (which represents >>>>>> `unknown`) as noted in [1]. A new test has been >>>>>> included in this commit to verify the change. Furthermore, this doesn't >>>>>> impact the `java.util.zip.GZIPInputStream` >>>>>> since it ignores [2] this header value while reading the headers from >>>>>> the stream. [1] >>>>>> https://tools.ietf.org/html/rfc1952#page-7 >>>>>> <https://tools.ietf.org/html/rfc1952#page-7> [2] >>>>>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173 >>>>>> >>>>>> <https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173> >>>>> Hi Jaikiran, >>>>> >>>>> The change seems fine an inline with the RFC. I can sponsor this once we >>>>> have another review. >>>>> >>>>> I have run the JCK tests for Zip/Gzip/Jar and Mach5 JDK tier1, tier2 and >>>>> tier3 >>>> Thank you Lance for the review and running the tests. I'll wait for >>>> another review before initiating the integrate >>>> command. >>>> >>>> - >>>> >>>> PR: https://git.openjdk.java.net/jdk/pull/130 >>>> <https://git.openjdk.java.net/jdk/pull/130> >> >> >> Best >> Lance >> -- >> >> >> >> >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> >> >> >> >> Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance) [v4]
On Tue, 15 Sep 2020 12:39:11 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for this patch which fixes the issue >> reported in >> https://bugs.openjdk.java.net/browse/JDK-8244706? >> The commit here sets the `OS` header flag to `255` (which represents >> `unknown`) as noted in [1]. A new test has been >> included in this commit to verify the change. Furthermore, this doesn't >> impact the `java.util.zip.GZIPInputStream` >> since it ignores [2] this header value while reading the headers from the >> stream. [1] >> https://tools.ietf.org/html/rfc1952#page-7 [2] >> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173 > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Remove unintended file commit Thank you for making the changes that Brent and I suggested. This looks good. I have submitted, a [CSR](https://bugs.openjdk.java.net/browse/JDK-8253142) to track the change. I have also created a [Release Note](https://bugs.openjdk.java.net/browse/JDK-8253185) as well. Once the CSR has been approved, I will sponsor your change - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/130
Re: RFR: 6714834: JarFile.getManifest() leaves an open InputStream as an undocumented side effect [v2]
On Tue, 15 Sep 2020 16:59:59 GMT, Lance Andersen wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove "final" > > I am fine with this as well. I will pull over the change and just sanity > check it via mach5 and then will sponsor @jaikiran Please go ahead and integrate this and I can then sponsor (has to be done in that order) - PR: https://git.openjdk.java.net/jdk/pull/186
Re: RFR: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
On Wed, 16 Sep 2020 17:29:52 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to the issue wrt missing hashCode() javadoc, which was > recently discussed in core-libs ml. Looks in line with the discussion on core-libs. - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/208
Re: RFR: 8253153: Mentioning of "hour-of-minute" in java.time.temporal.TemporalField JavaDoc
On Fri, 18 Sep 2020 01:49:09 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/234
RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary
Hi all, Please review the fix for JDK-8252739 which addresses an issue introduced by https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored the offset specified by Deflater.setDictionary. Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly as well as the java/util/zip and java/util/jar JCK tests. - Commit messages: - Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary Changes: https://git.openjdk.java.net/jdk/pull/269/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252739 Stats: 186 lines in 2 files changed: 185 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/269.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/269/head:pull/269 PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v2]
On Sun, 20 Sep 2020 18:29:20 GMT, Uwe Schindler wrote: >> Lance Andersen has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Additional updates to DeflaterDictionaryTests.java > > The tests with byte buffers (direct and heap) are not using offsets > (arrayOffset=0). The direct buffer test uses just a > series of 0-bytes, so incorrect offsets won't change result. There should be > real data copied into the direct buffer. Minor updates have been made to the tests - PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v2]
> Hi all, > > Please review the fix for JDK-8252739 which addresses an issue introduced by > https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored > the offset specified by > Deflater.setDictionary. Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly > as well as the java/util/zip and > java/util/jar JCK tests. Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: Additional updates to DeflaterDictionaryTests.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/269/files - new: https://git.openjdk.java.net/jdk/pull/269/files/7f3fd5dc..eaf0be8c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=00-01 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/269.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/269/head:pull/269 PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v3]
On Sun, 20 Sep 2020 20:42:30 GMT, Uwe Schindler wrote: >> Minor updates have been made to the tests > > Ok much better for the heap buffers. Many thanks. > > The direct buffers have now contents, but I think it should copy the whole > heap array into the byte buffer. After that > set position and limit and then call slice(). After that you have a slice of > the original direct buffer with offset. I have made additional updates to testByteBufferDirect to incorporate your suggestions - PR: https://git.openjdk.java.net/jdk/pull/269
Re: RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v3]
> Hi all, > > Please review the fix for JDK-8252739 which addresses an issue introduced by > https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored > the offset specified by > Deflater.setDictionary. Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly > as well as the java/util/zip and > java/util/jar JCK tests. Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: More updates to testByteBufferDirect in DeflaterDictionaryTests.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/269/files - new: https://git.openjdk.java.net/jdk/pull/269/files/eaf0be8c..aac03e3e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=269&range=01-02 Stats: 15 lines in 1 file changed: 7 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/269.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/269/head:pull/269 PR: https://git.openjdk.java.net/jdk/pull/269
