Re: RFR: 8186958: Need method to create pre-sized HashMap [v14]
On Sun, 10 Apr 2022 20:28:16 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 17 commits: > > - Merge branch 'master' into fix_8186958 > - variable nameToReferenceSize rename to moduleCount > - use (double) DEFAULT_LOAD_FACTOR instead of 0.75 > - drop CalculateHashMapCapacityTestJMH > - refine javadoc for LinkedHashMap#newLinkedHashMap > - revert changes in jdk.compile > - update usages of LinkedHashMap > - update usages of HashMap > - update codes > - update jmh > - ... and 7 more: > https://git.openjdk.java.net/jdk/compare/34914f12...7adc89c1 OK, the CSR is approved now. Regarding the load factor of 0.75, I'm not quite sure whether there is a rigorous justification for this value. It seems "traditional." It does seem likely that using a load factor of 0.75 will generate fewer collisions than a load factor of 1.0. It would be an interesting for a future investigation to see how much of a performance difference the load factor makes. In practice I think there are very few cases where it makes sense to use anything other than the default. In any case, there's no reason to change anything from the current default of 0.75. It occurs to me that `HashSet` and `LinkedHashSet` have the same "capacity" problem. It would be good to add static factory methods for them too. This is probably best done as a separate effort: see [JDK-8284780](https://bugs.openjdk.java.net/browse/JDK-8284780). I've done some work on add test cases for these new static factory methods, and I've also added API notes to the capacity-based constructors to link to the new factory methods. Note that even though these are javadoc changes, the API notes are non-normative and don't require an update to the CSR. See the last few commits on this branch: https://github.com/stuart-marks/jdk/commits/JDK-8186958-presized-HashMap If you think they're good, please pull them in. Regarding the scope of changes, the number of call sites that are changed is indeed spread rather too widely across the JDK. In order to keep the number of required reviewers small, I think we should trim down the call sites to a more manageable set. Specifically, I'd suggest **removing** from this PR the changes from the files in the following areas: * src/java.desktop * src/java.management * src/jdk.internal.vm.ci * src/jdk.jfr * src/jdk.management.jfr * src/jdk.management * src/utils/IdealGraphVisualizer After removing these, there will still be a fair number of call sites. Several of these have errors, so they'll be sufficient to show the value of the new APIs. After that I'll recompute and readjust labels to pull in the right set of reviewers. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/7928
Integrated: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. This pull request has now been integrated. Changeset: 5691a3b6 Author:Glavo Committer: Yi Yang URL: https://git.openjdk.java.net/jdk/commit/5691a3b6afcb3229ccd0e00d3a4ec9ccacc93182 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod 8284702: Add @since for java.time.LocalDate.EPOCH Reviewed-by: rriggs, bpb, iris, darcy, naoto - PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8284638: store skip buffers in InputStream Object [v2]
On Tue, 12 Apr 2022 22:15:22 GMT, XenoAmess wrote: > What subclasses of InputStream in the JDK do not override skip(n)? >From a peek, the majority of subclasses do not override `skip(long)`. Most >overrides are delegations or optimizations for random access structures; but >there are some that create their custom local variable skip buffers, usually >of size 512 or minimum of 512 and the skip size. These custom skip buffer ones >IMO should have their overrides removed. > Most sequential streams are open for a relatively short period of time, the > lifetime of the > memory for the buffer won't change the memory usage enough to notice. True, and with good use of try-with-resources, these instance fields' array allocations shouldn't be too much of a problem compared to allocation in every skip(long) call. > If the concern is about tying up memory then allocate the buffer once and don't resize it. Each resize consumes extra memory and gc cycles to reclaim the last buffer. > Use the requested size but at least nnn and at most MAX_SKIP_BUFFER_SIZE. Shouldn't be too problematic, as most skip usages in JDK as I see are skipping small number of bytes like 2 or 4, or like skipping over attributes of Java class files. A minimum skip buffer size isn't that helpful, as I don't think we often see skip calls with slowly incremental sizes. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v2]
On Tue, 12 Apr 2022 22:19:18 GMT, XenoAmess wrote:
>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add MIN_SKIP_BUFFER_SIZE
src/java.base/share/classes/java/io/InputStream.java line 557:
> 555:
> 556: while (remaining > 0) {
> 557: nr = read(skipBuffer, 0, (int)Math.min(size, remaining));
I recommend moving `nr` declaration from the beginning of the method to where
it's actually used (here)
-
PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v2]
On Tue, 12 Apr 2022 20:39:52 GMT, Roger Riggs wrote: > Without specific information about use cases, there isn't enough information > to craft a good algorithm/solution and simplicity is preferred. The > MAX_SKIP_BUFFER_SIZE is 2048 (not 8192). > > What subclasses of InputStream in the JDK do not override skip(n)? Most > sequential streams are open for a relatively short period of time, the > lifetime of the memory for the buffer won't change the memory usage enough to > notice. > > If the concern is about tying up memory then allocate the buffer once and > don't resize it. Each resize consumes extra memory and gc cycles to reclaim > the last buffer. Use the requested size but at least nnn and at most > MAX_SKIP_BUFFER_SIZE. @RogerRiggs Sounds reasonable and applied. Should we change the implementation in Reader class as well? - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v2]
> @jmehrens what about this then? > I think it safe now(actually this mechanism is learned from Reader) XenoAmess has updated the pull request incrementally with one additional commit since the last revision: add MIN_SKIP_BUFFER_SIZE - Changes: - all: https://git.openjdk.java.net/jdk/pull/5872/files - new: https://git.openjdk.java.net/jdk/pull/5872/files/e29f7483..81e9ca49 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=00-01 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5872/head:pull/5872 PR: https://git.openjdk.java.net/jdk/pull/5872
Withdrawn: 8284637: Improve String.join performance
On Fri, 8 Apr 2022 19:33:26 GMT, XenoAmess wrote: > 8284637: Improve String.join performance This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/8169
Re: RFR: 8284637: Improve String.join performance [v2]
On Mon, 11 Apr 2022 21:35:39 GMT, XenoAmess wrote: >> 8284637: Improve String.join performance > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > add jmh JMH results shows not worthy. closed. - PR: https://git.openjdk.java.net/jdk/pull/8169
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v5]
> A few additional enhancements aiming to improve VH performance in the > interpreter: > > - Flatten `TypeAndInvokers`: adds a pointer to `VarHandle` (a small increase > 40->48) but removes an object and an indirection on any instance actually > used - and might avoid allocating the `MethodHandle[]` unnecessarily on some > instances > - Have `checkExactAccessMode` return the directness of the `VarHandle` so > that we can avoid some `isDirect` method calls. > > Baseline, `-Xint` > > Benchmark Mode CntScore Error Units > VarHandleExact.exact_exactInvocation avgt 30 478.324 ? 5.762 ns/op > VarHandleExact.generic_exactInvocationavgt 30 392.114 ? 1.644 ns/op > VarHandleExact.generic_genericInvocation avgt 30 822.484 ? 1.865 ns/op > > > Patched, `-Xint` > > Benchmark Mode CntScore Error Units > VarHandleExact.exact_exactInvocation avgt 30 437.704 ? 5.320 ns/op > VarHandleExact.generic_exactInvocationavgt 30 374.512 ? 3.154 ns/op > VarHandleExact.generic_genericInvocation avgt 30 757.054 ? 1.237 ns/op > > > No significant performance difference in normal mode. Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: typo - Changes: - all: https://git.openjdk.java.net/jdk/pull/8160/files - new: https://git.openjdk.java.net/jdk/pull/8160/files/df1d652b..9998e538 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8160=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8160=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8160.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8160/head:pull/8160 PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v5]
On Tue, 12 Apr 2022 20:33:53 GMT, Naoto Sato wrote: >> Supporting `IsoFields` temporal fields in chronologies that are similar to >> ISO chronology. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 11 additional commits since > the last revision: > > - abstract class -> top level interface > - interface -> abstract class > - Merge branch 'master' into JDK-8279185 > - Removed the method > - Merge branch 'master' into JDK-8279185 > - Changed to use a type to determine ISO based or not > - Renamed the new method > - Merge branch 'master' into JDK-8279185 > - Addresses review comments > - copyright year fix > - ... and 1 more: > https://git.openjdk.java.net/jdk/compare/30616d77...7f596789 Looks good to me. For the name, another option might be IsoCompatible instead of IsoBased as historically those other calendars were established before the ISO standard, although technically, in the Java language, it could be said the xChronology is ISO based. - PR: https://git.openjdk.java.net/jdk/pull/7683
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v5]
On Tue, 12 Apr 2022 20:33:53 GMT, Naoto Sato wrote: >> Supporting `IsoFields` temporal fields in chronologies that are similar to >> ISO chronology. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 11 additional commits since > the last revision: > > - abstract class -> top level interface > - interface -> abstract class > - Merge branch 'master' into JDK-8279185 > - Removed the method > - Merge branch 'master' into JDK-8279185 > - Changed to use a type to determine ISO based or not > - Renamed the new method > - Merge branch 'master' into JDK-8279185 > - Addresses review comments > - copyright year fix > - ... and 1 more: > https://git.openjdk.java.net/jdk/compare/c751c9bd...7f596789 Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7683
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v4]
> A few additional enhancements aiming to improve VH performance in the > interpreter: > > - Flatten `TypeAndInvokers`: adds a pointer to `VarHandle` (a small increase > 40->48) but removes an object and an indirection on any instance actually > used - and might avoid allocating the `MethodHandle[]` unnecessarily on some > instances > - Have `checkExactAccessMode` return the directness of the `VarHandle` so > that we can avoid some `isDirect` method calls. > > Baseline, `-Xint` > > Benchmark Mode CntScore Error Units > VarHandleExact.exact_exactInvocation avgt 30 478.324 ? 5.762 ns/op > VarHandleExact.generic_exactInvocationavgt 30 392.114 ? 1.644 ns/op > VarHandleExact.generic_genericInvocation avgt 30 822.484 ? 1.865 ns/op > > > Patched, `-Xint` > > Benchmark Mode CntScore Error Units > VarHandleExact.exact_exactInvocation avgt 30 437.704 ? 5.320 ns/op > VarHandleExact.generic_exactInvocationavgt 30 374.512 ? 3.154 ns/op > VarHandleExact.generic_genericInvocation avgt 30 757.054 ? 1.237 ns/op > > > No significant performance difference in normal mode. Claes Redestad has updated the pull request incrementally with two additional commits since the last revision: - Improve javadoc for merged method - Add javadoc for merged method - Changes: - all: https://git.openjdk.java.net/jdk/pull/8160/files - new: https://git.openjdk.java.net/jdk/pull/8160/files/2a4fbd6d..df1d652b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8160=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8160=02-03 Stats: 10 lines in 1 file changed: 10 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8160.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8160/head:pull/8160 PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8284638: store skip buffers in InputStream Object
On Tue, 12 Apr 2022 18:00:18 GMT, XenoAmess wrote: >> Is this change proposed as the result of some experience with particular >> apps and streams or just observation of the implementation? >> >> Is there any information about what kind of underlying streams do not >> support skip directly >> and what is the distribution of sizes expected to be 'skipped'? >> GIven the object instance overhead of an array, I'd be inclined to suggest a >> minimum size that would >> cover all of the 'small' skipping that could occur. And if that's not big >> enough, jump to the MAX_SKIP_BUFFER_SIZE. To keep the code simple, I'd start >> at 128bytes and jump to the max if that's not big enough. There is precious >> little actual information available to fine tune. > >> Is this change proposed as the result of some experience with particular >> apps and streams or just observation of the implementation? > > just observation of the implementation of InputStream class and Reader class, > and somehow wonder why there be different handling in skip to these 2 classes. > >> Is there any information about what kind of underlying streams do not >> support skip directly and what is the distribution of sizes expected to be >> 'skipped'? GIven the object instance overhead of an array, I'd be inclined >> to suggest a minimum size that would cover all of the 'small' skipping that >> could occur. And if that's not big enough, jump to the MAX_SKIP_BUFFER_SIZE. >> To keep the code simple, I'd start at 128bytes and jump to the max if that's >> not big enough. There is precious little actual information available to >> fine tune. > > It whould make the problem @liach said above heavier. > > For example, if people just invoke 129, then the imput stream object hold a > 8192 length array until it die. > > That sounds much too memory wasting. Without specific information about use cases, there isn't enough information to craft a good algorithm/solution and simplicity is preferred. The MAX_SKIP_BUFFER_SIZE is 2048 (not 8192). What subclasses of InputStream in the JDK do not override skip(n)? Most sequential streams are open for a relatively short period of time, the lifetime of the memory for the buffer won't change the memory usage enough to notice. If the concern is about tying up memory then allocate the buffer once and don't resize it. Each resize consumes extra memory and gc cycles to reclaim the last buffer. Use the requested size but at least nnn and at most MAX_SKIP_BUFFER_SIZE. - PR: https://git.openjdk.java.net/jdk/pull/5872
Integrated: 8284771: java/util/zip/CloseInflaterDeflaterTest.java failed with "AssertionError: Expected IOException to be thrown, but nothing was thrown"
On Tue, 12 Apr 2022 20:09:31 GMT, Ravi Reddy wrote: > CloseInflaterDeflaterTest.java is failing intermittently(Observed once in > macOS and Linux), testInflaterOutputStream() is added as an extra test as > part of https://bugs.openjdk.java.net/browse/JDK-8278794. Disabling this test > for now before debugging any timing issues in Inflater. This pull request has now been integrated. Changeset: 7891085a Author:Ravi Reddy Committer: Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/7891085a877b8a5715d095e0c0dbaaf5bc8f16bb Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod 8284771: java/util/zip/CloseInflaterDeflaterTest.java failed with "AssertionError: Expected IOException to be thrown, but nothing was thrown" Reviewed-by: lancea - PR: https://git.openjdk.java.net/jdk/pull/8213
Re: RFR: 8284771: java/util/zip/CloseInflaterDeflaterTest.java failed with "AssertionError: Expected IOException to be thrown, but nothing was thrown"
On Tue, 12 Apr 2022 20:09:31 GMT, Ravi Reddy wrote: > CloseInflaterDeflaterTest.java is failing intermittently(Observed once in > macOS and Linux), testInflaterOutputStream() is added as an extra test as > part of https://bugs.openjdk.java.net/browse/JDK-8278794. Disabling this test > for now before debugging any timing issues in Inflater. Hi Ravi, Yes, makes sense to disable the test to keep Mach5 happy given this failure is very very intermittent while you try to chase down whether it is an implementation or test issue - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8213
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v5]
> Supporting `IsoFields` temporal fields in chronologies that are similar to > ISO chronology. Corresponding CSR has also been drafted. Naoto Sato has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision: - abstract class -> top level interface - interface -> abstract class - Merge branch 'master' into JDK-8279185 - Removed the method - Merge branch 'master' into JDK-8279185 - Changed to use a type to determine ISO based or not - Renamed the new method - Merge branch 'master' into JDK-8279185 - Addresses review comments - copyright year fix - ... and 1 more: https://git.openjdk.java.net/jdk/compare/488250f0...7f596789 - Changes: - all: https://git.openjdk.java.net/jdk/pull/7683/files - new: https://git.openjdk.java.net/jdk/pull/7683/files/530ed40e..7f596789 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7683=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7683=03-04 Stats: 302965 lines in 3965 files changed: 219208 ins; 39068 del; 44689 mod Patch: https://git.openjdk.java.net/jdk/pull/7683.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7683/head:pull/7683 PR: https://git.openjdk.java.net/jdk/pull/7683
RFR: 8284771: java/util/zip/CloseInflaterDeflaterTest.java failed with "AssertionError: Expected IOException to be thrown, but nothing was thrown"
CloseInflaterDeflaterTest.java is failing intermittently(Observed once in macOS and Linux), testInflaterOutputStream() is added as an extra test as part of https://bugs.openjdk.java.net/browse/JDK-8278794. Disabling this test for now before debugging any timing issues in Inflater. - Commit messages: - 8284771: java/util/zip/CloseInflaterDeflaterTest.java failed with AssertionError: Expected IOException to be thrown, but nothing was thrown Changes: https://git.openjdk.java.net/jdk/pull/8213/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8213=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284771 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8213.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8213/head:pull/8213 PR: https://git.openjdk.java.net/jdk/pull/8213
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Fri, 8 Apr 2022 13:43:39 GMT, Alan Bateman wrote:
> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
> JDK version to target.
>
> We will refresh this PR periodically to pick up changes and fixes from the
> loom repo.
>
> Most of the new mechanisms in the HotSpot VM are disabled by default and
> require running with `--enable-preview` to enable.
>
> The patch has support for x64 and aarch64 on the usual operating systems
> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for zero
> and some of the other ports. Additional ports can be contributed via PRs
> against the fibers branch in the loom repo.
>
> There are changes in many areas. To reduce notifications/mails, the labels
> have been trimmed down for now to hotspot, serviceability and core-libs.
> We'll add the complete set of labels when the PR is further along.
>
> The changes include a refresh of java.util.concurrent and ForkJoinPool from
> Doug Lea's CVS. These changes will probably be proposed and integrated in
> advance of this PR.
>
> The changes include some non-exposed and low-level infrastructure to support
> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to
> make life a bit easier and avoid having to separate VM changes and juggle
> branches at this time.
src/java.base/share/classes/sun/nio/cs/StreamEncoder.java line 110:
> 108:
> 109: public void flushBuffer() throws IOException {
> 110: if (lock instanceof InternalLock locker) {
now that StreamEncoder is final, we can drop the `else` branch
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v6]
On Tue, 12 Apr 2022 15:00:29 GMT, Volker Simonis wrote:
>> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)`
>> to highlight that it might write more bytes than the returned number of
>> inflated bytes into the buffer `b`.
>>
>> The superclass `java.io.InputStream` specifies that `read(byte[] b, int off,
>> int len)` will leave the content beyond the last read byte in the read
>> buffer `b` unaffected. However, the overridden `read` method in
>> `InflaterInputStream` passes the read buffer `b` to
>> `Inflater::inflate(byte[] b, int off, int len)` which doesn't provide this
>> guarantee. Depending on implementation details, `Inflater::inflate` might
>> write more than the returned number of inflated bytes into the buffer `b`.
>>
>> ### TL;DR
>>
>> `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater
>> functionality. `Inflater::inflate(byte[] output, int off, int len)`
>> currently calls zlib's native `inflate(..)` function and passes the address
>> of `output[off]` and `len` to it via JNI.
>>
>> The specification of zlib's `inflate(..)` function (i.e. the [API
>> documentation in the original zlib
>> implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400))
>> doesn't give any guarantees with regard to usage of the output buffer. It
>> only states that upon completion the function will return the number of
>> bytes that have been written (i.e. "inflated") into the output buffer.
>>
>> The original zlib implementation only wrote as many bytes into the output
>> buffer as it inflated. However, this is not a hard requirement and newer,
>> more performant implementations of the zlib library like
>> [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/)
>> or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes
>> of the output buffer than they actually inflate as a scratch buffer. See
>> https://github.com/simonis/zlib-chromium for a more detailed description of
>> their approach and its performance benefit.
>>
>> These new zlib versions can still be used transparently from Java (e.g. by
>> putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because
>> they still fully comply to specification of `Inflater::inflate(..)`.
>> However, we might run into problems when using the `Inflater` functionality
>> from the `InflaterInputStream` class. `InflaterInputStream` is derived from
>> from `InputStream` and as such, its `read(byte[] b, int off, int len)`
>> method is quite constrained. It specifically specifies that if *k* bytes
>> have been read, then "these bytes will be stored in elements `b[off]`
>> through `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through
>> `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] b, int
>> off, int len)` (which is constrained by `InputStream::read(..)`'s
>> specification) calls `Inflater::inflate(byte[] b, int off, int len)` and
>> directly passes its output buffer down to the native zlib `inflate(..)`
>> method which is free to change the bytes beyond `b[off+`
*k*`]` (where *k* is the number of inflated bytes).
>>
>> From a practical point of view, I don't see this as a big problem, because
>> callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never
>> know how many bytes will be written into the output buffer `b` (and in fact
>> its content can always be completely overwritten). It therefore makes no
>> sense to depend on any data there being untouched after the call. Also,
>> having used zlib-cloudflare productively for about two years, we haven't
>> seen real-world issues because of this behavior yet. However, from a
>> specification point of view it is easy to artificially construct a program
>> which violates `InflaterInputStream::read(..)`'s postcondition if using one
>> of the alterantive zlib implementations. A recently integrated JTreg test
>> (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails
>> with zlib-chromium but can fixed easily to run with alternative
>> implementations as well (see
>> [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)).
>
> Volker Simonis has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Adapted wording based on @AlanBateman's review, removed implementation note
> on ZipFile::getInputStream and aligned wording for all ::read methods
Hi Volker,
I think this reads much better. Its too bad we cannot take advantage of
`@inheritedDoc`
A couple of minor comments below
src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 99:
> 97: * no bytes are read and {@code 0} is returned.
> 98: *
> 99: * If n denotes the returned number of inflated bytes then
> {@code buf[off]}
I think a comma might be needed after "...inflated bytes"
Re: RFR: 8284638: store skip buffers in InputStream Object
On Mon, 11 Apr 2022 22:58:19 GMT, Roger Riggs wrote: > Is this change proposed as the result of some experience with particular apps > and streams or just observation of the implementation? just observation of the implementation of InputStream class and Reader class, and somehow wonder why there be different handling in skip to these 2 classes. > Is there any information about what kind of underlying streams do not support > skip directly and what is the distribution of sizes expected to be 'skipped'? > GIven the object instance overhead of an array, I'd be inclined to suggest a > minimum size that would cover all of the 'small' skipping that could occur. > And if that's not big enough, jump to the MAX_SKIP_BUFFER_SIZE. To keep the > code simple, I'd start at 128bytes and jump to the max if that's not big > enough. There is precious little actual information available to fine tune. It whould make the problem @liach said above heavier. For example, if people just invoke 129, then the imput stream object hold a 8192 length array until it die. That sounds much too memory wasting. - PR: https://git.openjdk.java.net/jdk/pull/5872
Integrated: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. This pull request has now been integrated. Changeset: 19b140a7 Author:Raffaello Giulietti Committer: Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/19b140a7f30ea600d66bcf8370d94f5d6bf6d0d1 Stats: 60 lines in 2 files changed: 51 ins; 0 del; 9 mod 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed Reviewed-by: jlaskey, bpb - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. j.u.Random and j.u.SplittableRandom do _not_ expose (byte[]) ctors. It's the factory that falls back to the () ctor if needed. A more sophisticated test could perhaps use reflection to discover which ctors are exposed. - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 17:30:07 GMT, Jim Laskey wrote: > All generators support the byte[] however many ignore the argument. "If > byte[] seed is not supported by an > [algorithm](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/util/random/package-summary.html#algorithms) > then the no argument form of create is used." Currently only the LXM > algorithms use byte[]. If other algorithms get added then the test can be > expanded. Sounds good; thanks for the additional context. - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. All generators support the byte[] however many ignore the argument. "If byte[] seed is not supported by an [algorithm](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/util/random/package-summary.html#algorithms) then the no argument form of create is used." Currently only the LXM algorithms use byte[]. If other algorithms get added then the test can be expanded. - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. Not all random generators expose a (byte[]) constructor. The test is limited to the LXM group because it is specific for the bug and these generators are known to offer such a constructor. There are others, though. However, I don't know how to query the factory for constructor parameter types. - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. Are there any tests for corresponding issues in classes for other algorithms? If not, that could be done as follow-up work. - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283870: jdeprscan --help causes an exception when the locale is ja, zh_CN or de [v2]
On Tue, 12 Apr 2022 06:36:29 GMT, Koichi Sakata wrote:
>> # Summary
>> Running jdeprscan with --help option causes an exception on any OSs when the
>> locale is ja, zh_CN or de.
>>
>> # How to reproduce this issue
>>
>> $ jdeprscan -J-Duser.language=ja --help
>> Exception in thread "main" java.lang.IllegalArgumentException: can't parse
>> argument number: dir|jar|class
>> at
>> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1454)
>> at
>> java.base/java.text.MessageFormat.applyPattern(MessageFormat.java:492)
>> at java.base/java.text.MessageFormat.(MessageFormat.java:371)
>> at java.base/java.text.MessageFormat.format(MessageFormat.java:860)
>> at jdk.jdeps/com.sun.tools.jdeprscan.Messages.get(Messages.java:62)
>> at jdk.jdeps/com.sun.tools.jdeprscan.Main.printHelp(Main.java:706)
>> at jdk.jdeps/com.sun.tools.jdeprscan.Main.run(Main.java:529)
>> at jdk.jdeps/com.sun.tools.jdeprscan.Main.call(Main.java:717)
>> at jdk.jdeps/com.sun.tools.jdeprscan.Main.main(Main.java:725)
>> Caused by: java.lang.NumberFormatException: For input string: "dir|jar|class"
>> at
>> java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
>> at java.base/java.lang.Integer.parseInt(Integer.java:668)
>> at java.base/java.lang.Integer.parseInt(Integer.java:786)
>> at
>> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1452)
>> ... 8 more
>>
>> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help
>> （Same as above）
>>
>> $ jdeprscan -J-Duser.language=de --help
>> （Same as above）
>>
>> Of course, it works well when the locale is anything other than those
>> locales.
>>
>> # Details
>> I found **''**{dir|jar|class}**''** in both ja and zh_CN properties files.
>> Doubled single quotes mean a single quote in MessageFormat class, so the
>> class recognizes {dir|jar|class} as a FormatElement and throws the
>> exception.
>>
>> I removed extra single quotes.
>>
>> # Test
>> It seems there is no test class for it. So I run commands as I mentioned
>> before.
>>
>> $ jdeprscan -J-Duser.language=ja --help
>> 使用方法: jdeprscan [options] {dir|jar|class} ...
>>
>> オプション:
>> --class-path PATH
>> --for-removal
>> --full-version
>> -? -h --help
>> -l--list
>> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
>> -v--verbose
>> --version
>>
>> 非推奨APIの使用について各引数をスキャンします。引数には、
>> パッケージ階層のルートを指定するディレクトリ、JARファイル、
>> クラス・ファイルまたはクラス名を使用できます。クラス名は、
>> 完全修飾クラス名を使用して指定する必要があります。ネストされた
>> クラスは$で区切ります。例:
>>
>> java.lang.Thread$State
>>
>> --class-pathオプションは、依存するクラスの解決のための
>> 検索パスを指定します。
>>
>> --for-removalオプションは、スキャンとリスト化を削除予定で非推奨のAPIに
>> 限定します。リリース値が6、7または8の場合は使用できません。
>>
>> --full-versionオプションはツールのバージョン文字列の全体を出力します。
>>
>> --help (-? -h)オプションは、ヘルプ・メッセージ全体を出力します。
>>
>> --list (-l)オプションは非推奨APIセットを出力します。スキャンは行われないため、
>> ディレクトリ、JARまたはクラス引数を指定する必要はありません。
>>
>> --releaseオプションは、スキャンする非推奨APIのセットを提供するJava SE
>> リリースを指定します。
>>
>> --verbose (-v)オプションを使用すると、処理中に追加のメッセージを出力できます。
>>
>> --versionオプションは、簡略化されたツールのバージョン文字列を出力します。
>>
>> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help
>> 用法：jdeprscan [options] {dir|jar|class} ...
>>
>> 选项：
>> --class-path PATH
>> --for-removal
>> --full-version
>> -? -h --help
>> -l--list
>> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
>> -v--verbose
>> --version
>>
>> 扫描每个参数以了解是否使用了过时的 API。
>> 参数可以是指定程序包分层结构、JAR 文件、
>> 类文件或类名的根的目录。类名必须
>> 使用全限定类名指定，并使用 $ 分隔符
>> 指定嵌套类，例如，
>>
>> java.lang.Thread$State
>>
>> --class-path 选项提供了用于解析从属类的
>> 搜索路径。
>>
>> --for-removal 选项限制扫描或列出已过时并待删除
>> 的 API。不能与发行版值 6、7 或 8 一起使用。
>>
>> --full-version 选项输出工具的完整版本字符串。
>>
>> --help (-? -h) 选项输出完整的帮助消息。
>>
>> --list (-l) 选项输出一组已过时的 API。不执行扫描，
>> 因此不应提供任何目录、jar 或类参数。
>>
>> --release 选项指定提供要扫描的已过时 API 集
>> 的 Java SE 发行版。
>>
>> --verbose (-v) 选项在处理期间启用附加消息输出。
>>
>> --version 选项输出工具的缩写版本字符串。
>>
>> $ jdeprscan -J-Duser.language=de --help
>> Verwendung: jdeprscan [Optionen] {dir|jar|class} ...
>>
>> Optionen:
>> --class-path PATH
>> --for-removal
>> --full-version
>> -? -h --help
>> -l--list
>> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
>> -v--verbose
>> --version
>>
>> Scannt jedes Argument auf die Verwendung veralteter APIs. Ein Argument
>> kann ein Verzeichnis, das die Root einer Packagehierarchie angibt,
>> eine JAR-Datei, eine Klassendatei oder ein Klassenname sein. Der Klassenname
>> muss durch einen vollständig qualifizierten Klassennamen mit $ als
>> Trennzeichen
>> für verschachtelte Klassen angegeben werden. Beispiel:
>>
>> java.lang.Thread$State
>>
>> Die Option --class-path liefert einen Suchpfad für die Auflösung
>> von abhängigen Klassen.
>>
>> Die Option --for-removal begrenzt das Scannen oder Auflisten auf APIs, die
>> veraltet sind
>> und entfernt
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Fri, 8 Apr 2022 13:43:39 GMT, Alan Bateman wrote:
> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
> JDK version to target.
>
> We will refresh this PR periodically to pick up changes and fixes from the
> loom repo.
>
> Most of the new mechanisms in the HotSpot VM are disabled by default and
> require running with `--enable-preview` to enable.
>
> The patch has support for x64 and aarch64 on the usual operating systems
> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for zero
> and some of the other ports. Additional ports can be contributed via PRs
> against the fibers branch in the loom repo.
>
> There are changes in many areas. To reduce notifications/mails, the labels
> have been trimmed down for now to hotspot, serviceability and core-libs.
> We'll add the complete set of labels when the PR is further along.
>
> The changes include a refresh of java.util.concurrent and ForkJoinPool from
> Doug Lea's CVS. These changes will probably be proposed and integrated in
> advance of this PR.
>
> The changes include some non-exposed and low-level infrastructure to support
> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to
> make life a bit easier and avoid having to separate VM changes and juggle
> branches at this time.
src/java.base/share/classes/sun/nio/ch/SelChImpl.java line 89:
> 87: } else {
> 88: long millis;
> 89: if (nanos == 0) {
Was this change intentional? (`<=` replaced by `==`)
If it was, please update the javadoc - `NANOSECONDS.toMillis(-1)` = 0 implies
no waiting in Net.poll
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. This highly nontrivial change really requires a lot of people to review! But it looks pretty legitimate to me. - Marked as reviewed by ice1...@github.com (no known OpenJDK username). PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
On Mon, 11 Apr 2022 10:13:40 GMT, Claes Redestad wrote: >> A few additional enhancements aiming to improve VH performance in the >> interpreter: >> >> - Flatten `TypeAndInvokers`: adds a pointer to `VarHandle` (a small increase >> 40->48) but removes an object and an indirection on any instance actually >> used - and might avoid allocating the `MethodHandle[]` unnecessarily on some >> instances >> - Have `checkExactAccessMode` return the directness of the `VarHandle` so >> that we can avoid some `isDirect` method calls. >> >> Baseline, `-Xint` >> >> Benchmark Mode CntScore Error Units >> VarHandleExact.exact_exactInvocation avgt 30 478.324 ? 5.762 ns/op >> VarHandleExact.generic_exactInvocationavgt 30 392.114 ? 1.644 ns/op >> VarHandleExact.generic_genericInvocation avgt 30 822.484 ? 1.865 ns/op >> >> >> Patched, `-Xint` >> >> Benchmark Mode CntScore Error Units >> VarHandleExact.exact_exactInvocation avgt 30 437.704 ? 5.320 ns/op >> VarHandleExact.generic_exactInvocationavgt 30 374.512 ? 3.154 ns/op >> VarHandleExact.generic_genericInvocation avgt 30 757.054 ? 1.237 ns/op >> >> >> No significant performance difference in normal mode. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > checkExactAccessMode -> checkAccessModeThenIsDirect, privatize throw > method, cleanup code generator Dropping `Exact` from `checkExactAccessMode` is good with me and it'd help future readers if you can add a javadoc what this method does. - PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. The @since tag was omitted as part of JDK-8143413 and it is good to add it now. - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. Marked as reviewed by jlaskey (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8207
RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
Please review this tiny fix. A test similar to the code proposed by the bug reporter has been added for the LXM group. It does not pass before the fix and passes after. - Commit messages: - 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed Changes: https://git.openjdk.java.net/jdk/pull/8207/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8207=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283083 Stats: 60 lines in 2 files changed: 51 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/8207.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8207/head:pull/8207 PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v6]
> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)` to > highlight that it might write more bytes than the returned number of > inflated bytes into the buffer `b`. > > The superclass `java.io.InputStream` specifies that `read(byte[] b, int off, > int len)` will leave the content beyond the last read byte in the read buffer > `b` unaffected. However, the overridden `read` method in > `InflaterInputStream` passes the read buffer `b` to `Inflater::inflate(byte[] > b, int off, int len)` which doesn't provide this guarantee. Depending on > implementation details, `Inflater::inflate` might write more than the > returned number of inflated bytes into the buffer `b`. > > ### TL;DR > > `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater > functionality. `Inflater::inflate(byte[] output, int off, int len)` currently > calls zlib's native `inflate(..)` function and passes the address of > `output[off]` and `len` to it via JNI. > > The specification of zlib's `inflate(..)` function (i.e. the [API > documentation in the original zlib > implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) > doesn't give any guarantees with regard to usage of the output buffer. It > only states that upon completion the function will return the number of bytes > that have been written (i.e. "inflated") into the output buffer. > > The original zlib implementation only wrote as many bytes into the output > buffer as it inflated. However, this is not a hard requirement and newer, > more performant implementations of the zlib library like > [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) > or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes > of the output buffer than they actually inflate as a scratch buffer. See > https://github.com/simonis/zlib-chromium for a more detailed description of > their approach and its performance benefit. > > These new zlib versions can still be used transparently from Java (e.g. by > putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because > they still fully comply to specification of `Inflater::inflate(..)`. However, > we might run into problems when using the `Inflater` functionality from the > `InflaterInputStream` class. `InflaterInputStream` is derived from from > `InputStream` and as such, its `read(byte[] b, int off, int len)` method is > quite constrained. It specifically specifies that if *k* bytes have been > read, then "these bytes will be stored in elements `b[off]` through > `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through `b[off+len-1]` > **unaffected**". But `InflaterInputStream::read(byte[] b, int off, int len)` > (which is constrained by `InputStream::read(..)`'s specification) calls > `Inflater::inflate(byte[] b, int off, int len)` and directly passes its > output buffer down to the native zlib `inflate(..)` method which is free to > change the bytes beyond `b[off+`* k*`]` (where *k* is the number of inflated bytes). > > From a practical point of view, I don't see this as a big problem, because > callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never > know how many bytes will be written into the output buffer `b` (and in fact > its content can always be completely overwritten). It therefore makes no > sense to depend on any data there being untouched after the call. Also, > having used zlib-cloudflare productively for about two years, we haven't seen > real-world issues because of this behavior yet. However, from a specification > point of view it is easy to artificially construct a program which violates > `InflaterInputStream::read(..)`'s postcondition if using one of the > alterantive zlib implementations. A recently integrated JTreg test > (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails > with zlib-chromium but can fixed easily to run with alternative > implementations as well (see > [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)). Volker Simonis has updated the pull request incrementally with one additional commit since the last revision: Adapted wording based on @AlanBateman's review, removed implementation note on ZipFile::getInputStream and aligned wording for all ::read methods - Changes: - all: https://git.openjdk.java.net/jdk/pull/7986/files - new: https://git.openjdk.java.net/jdk/pull/7986/files/1bf6bc1b..52524353 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7986=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7986=04-05 Stats: 14 lines in 4 files changed: 0 ins; 3 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/7986.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7986/head:pull/7986 PR: https://git.openjdk.java.net/jdk/pull/7986
RFR: 8284742: Handle integral division overflow during parsing
Hi, This patch moves the handling of integral division overflow on x86 from code emission time to parsing time. This allows the compiler to perform more efficient transformations and also aids in achieving better code layout. I also removed the handling for division by 10 in the ad file since it has been handled in `DivLNode::Ideal` already. Thank you very much. Before: Benchmark (BUFFER_SIZE) (divisorType) Mode Cnt Score Error Units IntegerDivMod.testDivide 1024 mixed avgt 5 2394.609 ± 66.460 ns/op IntegerDivMod.testDivide 1024 positive avgt 5 2411.390 ± 136.849 ns/op IntegerDivMod.testDivide 1024 negative avgt 5 2396.826 ± 57.079 ns/op IntegerDivMod.testDivideHoistedDivisor 1024 mixed avgt 5 2121.708 ± 17.194 ns/op IntegerDivMod.testDivideHoistedDivisor 1024 positive avgt 5 2118.761 ± 10.002 ns/op IntegerDivMod.testDivideHoistedDivisor 1024 negative avgt 5 2118.739 ± 22.626 ns/op IntegerDivMod.testDivideKnownPositive1024 mixed avgt 5 2467.937 ± 24.213 ns/op IntegerDivMod.testDivideKnownPositive1024 positive avgt 5 2463.659 ± 6.922 ns/op IntegerDivMod.testDivideKnownPositive1024 negative avgt 5 2480.384 ± 100.979 ns/op Benchmark (BUFFER_SIZE) (divisorType) Mode Cnt Score Error Units LongDivMod.testDivide1024 mixed avgt 5 8312.558 ± 18.408 ns/op LongDivMod.testDivide1024 positive avgt 5 8339.077 ± 127.893 ns/op LongDivMod.testDivide1024 negative avgt 5 8335.792 ± 160.274 ns/op LongDivMod.testDivideHoistedDivisor 1024 mixed avgt 5 7438.914 ± 17.948 ns/op LongDivMod.testDivideHoistedDivisor 1024 positive avgt 5 7550.720 ± 572.387 ns/op LongDivMod.testDivideHoistedDivisor 1024 negative avgt 5 7454.072 ± 70.805 ns/op LongDivMod.testDivideKnownPositive 1024 mixed avgt 5 12120.874 ± 82.832 ns/op LongDivMod.testDivideKnownPositive 1024 positive avgt 5 8898.518 ± 29.827 ns/op LongDivMod.testDivideKnownPositive 1024 negative avgt 5562.742 ± 2.795 ns/op After: Benchmark (BUFFER_SIZE) (divisorType) Mode Cnt Score Error Units IntegerDivMod.testDivide 1024 mixed avgt 5 2174.521 ± 13.054 ns/op IntegerDivMod.testDivide 1024 positive avgt 5 2172.389 ± 7.721 ns/op IntegerDivMod.testDivide 1024 negative avgt 5 2171.290 ± 12.902 ns/op IntegerDivMod.testDivideHoistedDivisor 1024 mixed avgt 5 2049.926 ± 29.098 ns/op IntegerDivMod.testDivideHoistedDivisor 1024 positive avgt 5 2043.896 ± 11.702 ns/op IntegerDivMod.testDivideHoistedDivisor 1024 negative avgt 5 2045.430 ± 17.232 ns/op IntegerDivMod.testDivideKnownPositive1024 mixed avgt 5 2281.506 ± 81.440 ns/op IntegerDivMod.testDivideKnownPositive1024 positive avgt 5 2279.727 ± 21.590 ns/op IntegerDivMod.testDivideKnownPositive1024 negative avgt 5 2275.898 ± 3.692 ns/op Benchmark (BUFFER_SIZE) (divisorType) Mode Cnt Score Error Units LongDivMod.testDivide1024 mixed avgt 5 8321.347 ± 93.932 ns/op LongDivMod.testDivide1024 positive avgt 5 8352.279 ± 213.565 ns/op LongDivMod.testDivide1024 negative avgt 5 8347.779 ± 203.612 ns/op LongDivMod.testDivideHoistedDivisor 1024 mixed avgt 5 7313.156 ± 113.426 ns/op LongDivMod.testDivideHoistedDivisor 1024 positive avgt 5 7299.939 ± 38.591 ns/op LongDivMod.testDivideHoistedDivisor 1024 negative avgt 5 7313.142 ± 100.068 ns/op LongDivMod.testDivideKnownPositive 1024 mixed avgt 5 9322.654 ± 276.328 ns/op LongDivMod.testDivideKnownPositive 1024 positive avgt 5 8639.404 ± 479.006 ns/op LongDivMod.testDivideKnownPositive 1024 negative avgt 5564.148 ± 6.009 ns/op - Commit messages: - comment grammar - x86_32 - add test cases
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v5]
On Mon, 11 Apr 2022 16:04:48 GMT, Alan Bateman wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Adapted wording and copyrights based on @jaikiran review > > src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 110: > >> 108: * @param len the maximum number of bytes read >> 109: * @return the actual number of bytes read, or -1 if the end of the >> 110: * compressed input stream is reached > > We should probably adjust the `@return` description, as was done for > InflaterInputStream, otherwise the method. description will say "the > returning number of inflated bytes" and the return description will say "the > actual number of bytes read". Fixed (also in `ZipInputStream`). - PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v5]
On Mon, 11 Apr 2022 16:07:15 GMT, Alan Bateman wrote:
>> Volker Simonis has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Adapted wording and copyrights based on @jaikiran review
>
> src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 103:
>
>> 101: * elements {@code buf[off+}n{@code ]} through {@code
>> buf[off+}len{@code -1]}
>> 102: * are undefined and an implementation is free to change them
>> during the inflate
>> 103: * operation. If the return value is -1 or an exception is thrown
>> the whole
>
> The sentence is very long. How about putting "an implementation is free ..."
> in parentheses after "undefined".
Done (also in `ZipInputStream` and `GZipInputStream` which both have the same
sentence).
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v5]
On Mon, 11 Apr 2022 16:05:33 GMT, Alan Bateman wrote:
>> Volker Simonis has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Adapted wording and copyrights based on @jaikiran review
>
> src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 104:
>
>> 102: * are undefined and an implementation is free to change them
>> during the inflate
>> 103: * operation. If the return value is -1 or an exception is thrown
>> the whole
>> 104: * content of {@code buf} is undefined.
>
> "whole content" doesn't seem right, it should be buf[off] to but[off + len -
> 1] is undefined.
Good catch. Fixed and also updated the corresponding part for
`InflaterInputStream::read`.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]
> This is a (preliminary) patch for javac implementation for the third preview > of pattern matching for switch (type patterns in switches). > > Draft JLS: > http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html > > The changes are: > -there are no guarded patterns anymore, guards are not bound to the > CaseElement (JLS 15.28) > -a new contextual keyword `when` is used to add a guard, instead of `&&` > -`null` selector value is handled on switch level (if a switch has `case > null`, it is used, otherwise a NPE is thrown), rather than on pattern > matching level. > -total patterns are allowed in `instanceof` > -`java.lang.MatchException` is added for the case where a switch is > exhaustive (due to sealed types) at compile-time, but not at runtime. > > Feedback is welcome! > > Thanks! Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision: Cleanup. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8182/files - new: https://git.openjdk.java.net/jdk/pull/8182/files/d7e2eb0a..ef7a7d6a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8182=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8182=01-02 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8182/head:pull/8182 PR: https://git.openjdk.java.net/jdk/pull/8182
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Mon, 11 Apr 2022 17:27:11 GMT, Daniel Fuchs wrote: > Not sure if that even matters - but there will be a slight change of > behaviour here if `InternalLock.CAN_USE_INTERNAL_LOCK` is ever `false`. > Instead of synchronizing on `in`, the `BufferedReader` will synchronize on > `this`. Good! We can change this so that depends on whether BufferedReader is extended and whether the given Reader is trusted. It's not clear if anyone could reliably depend on undocumented behavior like this but we have to be cautious at the same time. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8283689: Update the foreign linker VM implementation [v3]
> Hi, > > This PR updates the VM implementation of the foreign linker, by bringing over > commits from the panama-foreign repo. > > This is split off from the main JEP integration for 19, since we have limited > resources to handle this. As such, this PR might fall over to 20. > > I've written up an overview of the Linker architecture here: > http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful > to read that first. > > This patch moves from the "legacy" implementation, to what is currently > implemented in the panama-foreign repo, except for replacing the use of > method handle combinators with ASM. That will come in a later path. To recap. > This PR contains the following changes: > > 1. VM stubs for downcalls are now generated up front, instead of lazily by C2 > [1]. > 2. the VM support for upcalls/downcalls now support all possible call shapes. > And VM stubs and Java code implementing the buffered invocation strategy has > been removed [2], [3], [4], [5]. > 3. The existing C2 intrinsification support for the `linkToNative` method > handle linker was no longer needed and has been removed [6] (support might be > re-added in another form later). > 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now > implements RuntimeBlob directly. Binding to java classes has been rewritten > to use javaClasses.h/cpp (this wasn't previously possible due to these java > classes being in an incubator module) [7], [8], [9]. > > While the patch mostly consists of VM changes, there are also some Java > changes to support (2). > > The original commit structure has been mostly retained, so it might be useful > to look at a specific commit, or the corresponding patch in the > [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr) > repo as well. I've also left some inline comments to explain some of the > changes, which will hopefully make reviewing easier. > > Testing: Tier1-4 > > Thanks, > Jorn > > [1]: > https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358 > [2]: > https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49 > [3]: > https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3 > [4]: > https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3 > [5]: > https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120 > [6]: > https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a > [7]: > https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062 > [8]: > https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f > [9]: > https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Remove unneeded ComputeMoveOrder - Changes: - all: https://git.openjdk.java.net/jdk/pull/7959/files - new: https://git.openjdk.java.net/jdk/pull/7959/files/3434deda..a7b9f131 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7959=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7959=01-02 Stats: 174 lines in 1 file changed: 0 ins; 174 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7959.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7959/head:pull/7959 PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v27]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Add @ForceInline annotation on session accessor Beef up UnrolledAccess benchmark - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/a68195ae..66cebe77 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=26 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=25-26 Stats: 32 lines in 2 files changed: 32 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v2]
> This is a (preliminary) patch for javac implementation for the third preview > of pattern matching for switch (type patterns in switches). > > Draft JLS: > http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html > > The changes are: > -there are no guarded patterns anymore, guards are not bound to the > CaseElement (JLS 15.28) > -a new contextual keyword `when` is used to add a guard, instead of `&&` > -`null` selector value is handled on switch level (if a switch has `case > null`, it is used, otherwise a NPE is thrown), rather than on pattern > matching level. > -total patterns are allowed in `instanceof` > -`java.lang.MatchException` is added for the case where a switch is > exhaustive (due to sealed types) at compile-time, but not at runtime. > > Feedback is welcome! > > Thanks! Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision: Fixing tests. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8182/files - new: https://git.openjdk.java.net/jdk/pull/8182/files/da8401d4..d7e2eb0a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8182=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8182=00-01 Stats: 10 lines in 5 files changed: 5 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8182/head:pull/8182 PR: https://git.openjdk.java.net/jdk/pull/8182
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v6]
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 12 commits: > > - 4511638: Double.toString(double) sometimes produces incorrect results > >Adapted hashes in ElementStructureTest.java > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > >Enhanced intervals in MathUtils. >Updated references to Schubfach v4. > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > >Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) > comments. > - 4511638: Double.toString(double) sometimes produces incorrect results > >Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > - 4511638: Double.toString(double) sometimes produces incorrect results > >Refactored test classes to better match OpenJDK conventions. >Added tests recommended by Guy Steele and Paul Zimmermann. > - Changed MAX_CHARS to static > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76 Thanks Paul, really appreciate. - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8275535: Retrying a failed authentication on multiple LDAP servers can lead to users blocked
On Tue, 12 Apr 2022 01:17:37 GMT, Andrew John Hughes wrote: > What's the status of this? The last comment reads as if this is good to go. I believe we're still waiting for Martin to reply to the last comment: > Maybe, you could also log a bug for a test addition and describe in it an > environment, and sort of a high-level scenario of how JDK-8275535 can be > reproduced. Otherwise yes - this would be good to go. - PR: https://git.openjdk.java.net/jdk/pull/6043
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v6]
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 12 commits: > > - 4511638: Double.toString(double) sometimes produces incorrect results > >Adapted hashes in ElementStructureTest.java > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > >Enhanced intervals in MathUtils. >Updated references to Schubfach v4. > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > >Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) > comments. > - 4511638: Double.toString(double) sometimes produces incorrect results > >Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > - 4511638: Double.toString(double) sometimes produces incorrect results > >Refactored test classes to better match OpenJDK conventions. >Added tests recommended by Guy Steele and Paul Zimmermann. > - Changed MAX_CHARS to static > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76 for the record, I post here the review we did with Jean-Michel Muller of the Schubfach algorithm in last October [review.pdf](https://github.com/openjdk/jdk/files/8471359/review.pdf) - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
On Tue, 12 Apr 2022 01:15:33 GMT, David Holmes wrote: > > checkExactAccessMode -> checkAccessModeThenIsDirect > > Don't you still want "Exact" in there? That "access" check seems odd anyway > as it only checks for one form of mismatch - should it not also check for > `!exact && accessModeType(ad.type) == ad.symbolicMethodTypeExact`? What's checked is that the access mode is nominally OK. It just so happens that the only rule validated up front is that iff the VH is exact then the types must be an exact match. For non-exact VH sufficient checks will be done later, eg. by the `asType` call (if the type of the VH isn't adaptable to `ad.symbolicMethodTypeInvoker` this will throw an exception). With that in mind then the name `checkExactAccessMode` even appears misleading as it can be interpreted that the VH _must_ be `exact`. Which is not the case. Dropping the `Exact` part makes it less ambiguous. - PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v5]
On Tue, 12 Apr 2022 07:10:54 GMT, Jaikiran Pai wrote:
>> src/java.base/share/classes/java/util/zip/ZipFile.java line 347:
>>
>>> 345: *
>>> 346: * @implNote This implementation returns an instance of
>>> 347: * {@link java.util.zip.InflaterInputStream}.
>>
>> What is the reasoning for this note, do we really want it in the API docs?
>
> Hello Alan, this change was done by Volker for the point that I raised in the
> comment here https://github.com/openjdk/jdk/pull/7986#issuecomment-1081319691
> Hello Alan, this change was done by Volker for the point that I raised in the
> comment here [#7986
> (comment)](https://github.com/openjdk/jdk/pull/7986#issuecomment-1081319691)
I think it creates the temptation to cast the returned input stream to
InflaterInputStream, it might be better to leave it out.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v5]
On Mon, 11 Apr 2022 16:03:08 GMT, Alan Bateman wrote:
>> Volker Simonis has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Adapted wording and copyrights based on @jaikiran review
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 347:
>
>> 345: *
>> 346: * @implNote This implementation returns an instance of
>> 347: * {@link java.util.zip.InflaterInputStream}.
>
> What is the reasoning for this note, do we really want it in the API docs?
Hello Alan, this change was done by Volker for the point that I raised in the
comment here https://github.com/openjdk/jdk/pull/7986#issuecomment-1081319691
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8283870: jdeprscan --help causes an exception when the locale is ja, zh_CN or de
On Mon, 11 Apr 2022 05:31:49 GMT, Koichi Sakata wrote:
> # Summary
> Running jdeprscan with --help option causes an exception on any OSs when the
> locale is ja, zh_CN or de.
>
> # How to reproduce this issue
>
> $ jdeprscan -J-Duser.language=ja --help
> Exception in thread "main" java.lang.IllegalArgumentException: can't parse
> argument number: dir|jar|class
> at
> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1454)
> at
> java.base/java.text.MessageFormat.applyPattern(MessageFormat.java:492)
> at java.base/java.text.MessageFormat.(MessageFormat.java:371)
> at java.base/java.text.MessageFormat.format(MessageFormat.java:860)
> at jdk.jdeps/com.sun.tools.jdeprscan.Messages.get(Messages.java:62)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.printHelp(Main.java:706)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.run(Main.java:529)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.call(Main.java:717)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.main(Main.java:725)
> Caused by: java.lang.NumberFormatException: For input string: "dir|jar|class"
> at
> java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
> at java.base/java.lang.Integer.parseInt(Integer.java:668)
> at java.base/java.lang.Integer.parseInt(Integer.java:786)
> at
> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1452)
> ... 8 more
>
> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help
> （Same as above）
>
> $ jdeprscan -J-Duser.language=de --help
> （Same as above）
>
> Of course, it works well when the locale is anything other than those locales.
>
> # Details
> I found **''**{dir|jar|class}**''** in both ja and zh_CN properties files.
> Doubled single quotes mean a single quote in MessageFormat class, so the
> class recognizes {dir|jar|class} as a FormatElement and throws the exception.
>
> I removed extra single quotes.
>
> # Test
> It seems there is no test class for it. So I run commands as I mentioned
> before.
>
> $ jdeprscan -J-Duser.language=ja --help
> 使用方法: jdeprscan [options] {dir|jar|class} ...
>
> オプション:
> --class-path PATH
> --for-removal
> --full-version
> -? -h --help
> -l--list
> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
> -v--verbose
> --version
>
> 非推奨APIの使用について各引数をスキャンします。引数には、
> パッケージ階層のルートを指定するディレクトリ、JARファイル、
> クラス・ファイルまたはクラス名を使用できます。クラス名は、
> 完全修飾クラス名を使用して指定する必要があります。ネストされた
> クラスは$で区切ります。例:
>
> java.lang.Thread$State
>
> --class-pathオプションは、依存するクラスの解決のための
> 検索パスを指定します。
>
> --for-removalオプションは、スキャンとリスト化を削除予定で非推奨のAPIに
> 限定します。リリース値が6、7または8の場合は使用できません。
>
> --full-versionオプションはツールのバージョン文字列の全体を出力します。
>
> --help (-? -h)オプションは、ヘルプ・メッセージ全体を出力します。
>
> --list (-l)オプションは非推奨APIセットを出力します。スキャンは行われないため、
> ディレクトリ、JARまたはクラス引数を指定する必要はありません。
>
> --releaseオプションは、スキャンする非推奨APIのセットを提供するJava SE
> リリースを指定します。
>
> --verbose (-v)オプションを使用すると、処理中に追加のメッセージを出力できます。
>
> --versionオプションは、簡略化されたツールのバージョン文字列を出力します。
>
> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help
> 用法：jdeprscan [options] {dir|jar|class} ...
>
> 选项：
> --class-path PATH
> --for-removal
> --full-version
> -? -h --help
> -l--list
> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
> -v--verbose
> --version
>
> 扫描每个参数以了解是否使用了过时的 API。
> 参数可以是指定程序包分层结构、JAR 文件、
> 类文件或类名的根的目录。类名必须
> 使用全限定类名指定，并使用 $ 分隔符
> 指定嵌套类，例如，
>
> java.lang.Thread$State
>
> --class-path 选项提供了用于解析从属类的
> 搜索路径。
>
> --for-removal 选项限制扫描或列出已过时并待删除
> 的 API。不能与发行版值 6、7 或 8 一起使用。
>
> --full-version 选项输出工具的完整版本字符串。
>
> --help (-? -h) 选项输出完整的帮助消息。
>
> --list (-l) 选项输出一组已过时的 API。不执行扫描，
> 因此不应提供任何目录、jar 或类参数。
>
> --release 选项指定提供要扫描的已过时 API 集
> 的 Java SE 发行版。
>
> --verbose (-v) 选项在处理期间启用附加消息输出。
>
> --version 选项输出工具的缩写版本字符串。
>
> $ jdeprscan -J-Duser.language=de --help
> Verwendung: jdeprscan [Optionen] {dir|jar|class} ...
>
> Optionen:
> --class-path PATH
> --for-removal
> --full-version
> -? -h --help
> -l--list
> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
> -v--verbose
> --version
>
> Scannt jedes Argument auf die Verwendung veralteter APIs. Ein Argument
> kann ein Verzeichnis, das die Root einer Packagehierarchie angibt,
> eine JAR-Datei, eine Klassendatei oder ein Klassenname sein. Der Klassenname
> muss durch einen vollständig qualifizierten Klassennamen mit $ als
> Trennzeichen
> für verschachtelte Klassen angegeben werden. Beispiel:
>
> java.lang.Thread$State
>
> Die Option --class-path liefert einen Suchpfad für die Auflösung
> von abhängigen Klassen.
>
> Die Option --for-removal begrenzt das Scannen oder Auflisten auf APIs, die
> veraltet sind
> und entfernt werden sollen. Kann nicht mit den Releasewerten 6, 7, oder 8
> verwendet werden.
>
> Die Option --full-version gibt die vollständige Versionszeichenfolge des
> Tools
Re: RFR: 8283870: jdeprscan --help causes an exception when the locale is ja, zh_CN or de [v2]
> # Summary
> Running jdeprscan with --help option causes an exception on any OSs when the
> locale is ja, zh_CN or de.
>
> # How to reproduce this issue
>
> $ jdeprscan -J-Duser.language=ja --help
> Exception in thread "main" java.lang.IllegalArgumentException: can't parse
> argument number: dir|jar|class
> at
> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1454)
> at
> java.base/java.text.MessageFormat.applyPattern(MessageFormat.java:492)
> at java.base/java.text.MessageFormat.(MessageFormat.java:371)
> at java.base/java.text.MessageFormat.format(MessageFormat.java:860)
> at jdk.jdeps/com.sun.tools.jdeprscan.Messages.get(Messages.java:62)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.printHelp(Main.java:706)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.run(Main.java:529)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.call(Main.java:717)
> at jdk.jdeps/com.sun.tools.jdeprscan.Main.main(Main.java:725)
> Caused by: java.lang.NumberFormatException: For input string: "dir|jar|class"
> at
> java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
> at java.base/java.lang.Integer.parseInt(Integer.java:668)
> at java.base/java.lang.Integer.parseInt(Integer.java:786)
> at
> java.base/java.text.MessageFormat.makeFormat(MessageFormat.java:1452)
> ... 8 more
>
> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help
> （Same as above）
>
> $ jdeprscan -J-Duser.language=de --help
> （Same as above）
>
> Of course, it works well when the locale is anything other than those locales.
>
> # Details
> I found **''**{dir|jar|class}**''** in both ja and zh_CN properties files.
> Doubled single quotes mean a single quote in MessageFormat class, so the
> class recognizes {dir|jar|class} as a FormatElement and throws the exception.
>
> I removed extra single quotes.
>
> # Test
> It seems there is no test class for it. So I run commands as I mentioned
> before.
>
> $ jdeprscan -J-Duser.language=ja --help
> 使用方法: jdeprscan [options] {dir|jar|class} ...
>
> オプション:
> --class-path PATH
> --for-removal
> --full-version
> -? -h --help
> -l--list
> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
> -v--verbose
> --version
>
> 非推奨APIの使用について各引数をスキャンします。引数には、
> パッケージ階層のルートを指定するディレクトリ、JARファイル、
> クラス・ファイルまたはクラス名を使用できます。クラス名は、
> 完全修飾クラス名を使用して指定する必要があります。ネストされた
> クラスは$で区切ります。例:
>
> java.lang.Thread$State
>
> --class-pathオプションは、依存するクラスの解決のための
> 検索パスを指定します。
>
> --for-removalオプションは、スキャンとリスト化を削除予定で非推奨のAPIに
> 限定します。リリース値が6、7または8の場合は使用できません。
>
> --full-versionオプションはツールのバージョン文字列の全体を出力します。
>
> --help (-? -h)オプションは、ヘルプ・メッセージ全体を出力します。
>
> --list (-l)オプションは非推奨APIセットを出力します。スキャンは行われないため、
> ディレクトリ、JARまたはクラス引数を指定する必要はありません。
>
> --releaseオプションは、スキャンする非推奨APIのセットを提供するJava SE
> リリースを指定します。
>
> --verbose (-v)オプションを使用すると、処理中に追加のメッセージを出力できます。
>
> --versionオプションは、簡略化されたツールのバージョン文字列を出力します。
>
> $ jdeprscan -J-Duser.language=zh -J-Duser.country=CN --help
> 用法：jdeprscan [options] {dir|jar|class} ...
>
> 选项：
> --class-path PATH
> --for-removal
> --full-version
> -? -h --help
> -l--list
> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
> -v--verbose
> --version
>
> 扫描每个参数以了解是否使用了过时的 API。
> 参数可以是指定程序包分层结构、JAR 文件、
> 类文件或类名的根的目录。类名必须
> 使用全限定类名指定，并使用 $ 分隔符
> 指定嵌套类，例如，
>
> java.lang.Thread$State
>
> --class-path 选项提供了用于解析从属类的
> 搜索路径。
>
> --for-removal 选项限制扫描或列出已过时并待删除
> 的 API。不能与发行版值 6、7 或 8 一起使用。
>
> --full-version 选项输出工具的完整版本字符串。
>
> --help (-? -h) 选项输出完整的帮助消息。
>
> --list (-l) 选项输出一组已过时的 API。不执行扫描，
> 因此不应提供任何目录、jar 或类参数。
>
> --release 选项指定提供要扫描的已过时 API 集
> 的 Java SE 发行版。
>
> --verbose (-v) 选项在处理期间启用附加消息输出。
>
> --version 选项输出工具的缩写版本字符串。
>
> $ jdeprscan -J-Duser.language=de --help
> Verwendung: jdeprscan [Optionen] {dir|jar|class} ...
>
> Optionen:
> --class-path PATH
> --for-removal
> --full-version
> -? -h --help
> -l--list
> --release 7|8|9|10|11|12|13|14|15|16|17|18|19
> -v--verbose
> --version
>
> Scannt jedes Argument auf die Verwendung veralteter APIs. Ein Argument
> kann ein Verzeichnis, das die Root einer Packagehierarchie angibt,
> eine JAR-Datei, eine Klassendatei oder ein Klassenname sein. Der Klassenname
> muss durch einen vollständig qualifizierten Klassennamen mit $ als
> Trennzeichen
> für verschachtelte Klassen angegeben werden. Beispiel:
>
> java.lang.Thread$State
>
> Die Option --class-path liefert einen Suchpfad für die Auflösung
> von abhängigen Klassen.
>
> Die Option --for-removal begrenzt das Scannen oder Auflisten auf APIs, die
> veraltet sind
> und entfernt werden sollen. Kann nicht mit den Releasewerten 6, 7, oder 8
> verwendet werden.
>
> Die Option --full-version gibt die vollständige Versionszeichenfolge des
> Tools aus.
>
> Die Option --help (-? -h) gibt eine vollständige
