Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. Marked as reviewed by jpai (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8938
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v2]
> When generating `MethodHandle`-based concatenation expressions in > `StringConcatFactory` we can reduce the number of classes generated at > runtime by creating small batches of prependers and mixers before binding > them into the root expression tree. > > Improvements on one-off tests are modest, while the improvement on > bootstrapping stress tests can be substantial > ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): > > | Build |# classes| Runtime | > | --- | - | --- | > | Baseline | 31119 | 2.942s | > | Patch | 16208 | 1.958s | > > An earlier version of this patch saw a regression in the > `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along > with the optimizations in #8881 and #8923 that is no longer the case, and > allocation pressure is down slightly compared to the baseline on such a > repeat-the-same-bootstrap stress test: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2170.039 ? 117.441 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3538.020 ?4.558B/op > > This patch: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2144.807 ? 162.685 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3184.943 ?4.495B/op Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Minor stylistic cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/8855/files - new: https://git.openjdk.java.net/jdk/pull/8855/files/114d3fd8..263db625 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8855=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8855=00-01 Stats: 26 lines in 1 file changed: 6 ins; 10 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/8855.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8855/head:pull/8855 PR: https://git.openjdk.java.net/jdk/pull/8855
Re: [crac] RFR: Ensure empty Reference Handler and Cleaners queues
Could you please look at the updated version at [0]? The new API for ReferenceQueue still targets the problem of synchronizing Reference handling with CRaC. An example of that is a java object that becomes unreachable just before the checkpoint, and an associated Reference needs to be processed to release some native resource. Creating an image of the VM with that native resource linked is both unsafe (the native resource may not exist at the restore -- CRaC VM does its best to prevent successful checkpoint in this case) and inefficient, as every restored instance will perform the same processing of the same Reference that was captured by the image. So we need to ensure Reference processing is complete. For the processing done by a thread (or a set of threads), the change provides an updated API to await the set of threads blocked on the Queue awaiting references. This ensures that threads are done processing References from that Queue. > Once the method returns then there is no guarantee that the number > of waiters hasn't changed, but I think you know that I hoped to guarantee all Queues are empty by waiting a sufficient number of waiters for each Queue, in the order of Queues passing References between each other (for a single thread). But now even there, I see handling of a Reference later in the order may make another one pending, filling up a Queue that was supposed to be empty. For a strong guarantee that all Queues are empty, some sort of iteration may be required, that will check no Queue had a new reference since the last check. Processing of a single Reference may generate an arbitrary number of more enqueued References. More formally, ReferenceQueues and their processors form a directed graph, in which nodes are Queues and edges are relation "handling of a Reference from the source may enqueue another Reference into destination". Edges are defined by the code of processing and not data. The graph can be of the arbitrary form, e.g. there can be cycles, so Reference processing does not need even to converge. So the only reasonable way to get reference processing quiescent is to ensure References for each Queue are processed (by calling the new API), in the order of Queues may get References. I think a public API is needed as users may have the same problem as we do. But the current code does not support this (we need to allow user code after JDK Queues are emptied). The API now fully supports calling from the user code. Each invocation of the new API ensures all unreachable objects are discovered and pushed to a Queue before the Queue is checked for pending references and the number of waiting threads. > At a high level it should be okay to provide a JDK-internal way to > await quiescent. You've added it as a public API which might be okay > for the current exploration but I don't think it would be exposed in > its current form. The new ReferenceQueue API is moved into jdk.crac.* package, to avoid polluting Java API of CRaC EA builds that are based on JDK 17 for now. [0] https://github.com/openjdk/crac/pull/22 [1] https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/WeakHashMap.java#L361 Thanks, Anton
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v4]
> This is the implementation of JEP 428: Structured Concurrency (Incubator).
>
> This is a non-final API that provides a gentle on-ramp to structure a task as
> a family of concurrent subtasks, and to coordinate the subtasks as a unit.
Alan Bateman 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 10 additional commits since the
last revision:
- Fix typo in javadoc
- Merge
- Javadoc updates, add to jdk_loom test group
- Add statement to close about thread termination
- Use {@code ...}, replace task->subtask, fix typos, add jls ref
- Merge
- @ignore StructuredThreadDumpTest until test infra in place
- Refresh
- Merge
- Initial commit
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8787/files
- new: https://git.openjdk.java.net/jdk/pull/8787/files/4fc454a9..d11b24bc
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=8787=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=02-03
Stats: 33428 lines in 507 files changed: 6180 ins; 25559 del; 1689 mod
Patch: https://git.openjdk.java.net/jdk/pull/8787.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787
PR: https://git.openjdk.java.net/jdk/pull/8787
Integrated: 8287430 MemorySessionImpl::addOrCleanupIfFail does not rethrow exceptions
On Fri, 27 May 2022 10:29:14 GMT, Maurizio Cimadamore wrote: > This patch fix a missing rethrow in `MemorySessionImpl::addOrCleanupIfFail`. > As noted in the JBS issue, this bug does not affect correctness, but it > delays error reporting. > > Writing a test for this is nearly impossible, given that (a) a memory > resource created against a closed session would be inaccessible by clients > (because the session is closed!), and (b) because of the narrow window in > which the problem might manifest (for this problem to occur, a session state > change would have to occur between the first state check and when the cleanup > action list is updated). This pull request has now been integrated. Changeset: 0c420e03 Author:Maurizio Cimadamore URL: https://git.openjdk.java.net/jdk/commit/0c420e03ae24144a8146edb39f546841da33e381 Stats: 9 lines in 1 file changed: 3 ins; 0 del; 6 mod 8287430: MemorySessionImpl::addOrCleanupIfFail does not rethrow exceptions Reviewed-by: jvernee - PR: https://git.openjdk.java.net/jdk/pull/8917
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v4]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with two additional commits since the last revision: - Merge branch '8285838' of github.com:Deigue/jdk into 8285838 - Merge branch '8285838' of github.com:Deigue/jdk into 8285838 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/b80af8bf..d7831659 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8938
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v3]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with two additional commits since the last revision: - Merge branch '8285838' of github.com:Deigue/jdk into 8285838 - 8285838: Revised changes for Custom TZ DST test fixes. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/41e75494..b80af8bf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=01-02 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v2]
On Tue, 17 May 2022 20:19:28 GMT, Naoto Sato wrote: >> Gaurav Chaudhari has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8285838: Revised changes for Custom TZ DST test fixes. > > src/java.base/unix/native/libjava/TimeZone_md.c line 609: > >> 607: } >> 608: >> 609: offset = (gmt.tm_hour - localtm.tm_hour)*3600 + (gmt.tm_min - >> localtm.tm_min)*60; > > Since it is not using `timezone` anymore, we can reverse the subtraction, > i.e., `localtime` - `gmtime` so that the weird sign switch below can be > eliminated. I've reversed it and reverted the weird sign switching below this code snippet. > test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 2: > >> 1: /* >> 2: * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights >> reserved. > > This is a new test case, the year should be only 2022. Thanks, corrected this in the following commit > test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 1: > >> 1: # > > I'd change this script into a java test case (using `.sh` is not > recommended). In fact, this causes a failure invoking `javac` with `-Xmx768m` > (from TESTVMOPTS) > There are libraries under `jdk/test/lib/` for building test jvm process and > tools. I did away with this .sh script and combined the shell script and java test into one. Initially the only purpose of this shell script was in order to set up the environment variable for the proceeding test. However, I learnt that all this could be done within java, and I consolidated both the files into one test now. > test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 40: > >> 38: >> 39: OS=`uname -s` >> 40: case "$OS" in > > In case other than Linux/AIX, it would be helpful to emit some message that > the test is ignored. All moved to one java test now. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v2]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with one additional commit since the last revision: 8285838: Revised changes for Custom TZ DST test fixes. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/dcf762a5..41e75494 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=00-01 Stats: 94 lines in 3 files changed: 20 ins; 62 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches
On Mon, 23 May 2022 20:03:05 GMT, Claes Redestad wrote: > When generating `MethodHandle`-based concatenation expressions in > `StringConcatFactory` we can reduce the number of classes generated at > runtime by creating small batches of prependers and mixers before binding > them into the root expression tree. > > Improvements on one-off tests are modest, while the improvement on > bootstrapping stress tests can be substantial > ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): > > | Build |# classes| Runtime | > | --- | - | --- | > | Baseline | 31119 | 2.942s | > | Patch | 16208 | 1.958s | > > An earlier version of this patch saw a regression in the > `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along > with the optimizations in #8881 and #8923 that is no longer the case, and > allocation pressure is down slightly compared to the baseline on such a > repeat-the-same-bootstrap stress test: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2170.039 ? 117.441 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3538.020 ?4.558B/op > > This patch: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2144.807 ? 162.685 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3184.943 ?4.495B/op Before moving forward with this I've been looking at slightly more expensive bootstrapping of already known shapes. On one such bootstrap microbenchmark, `StringConcatFactoryBootstraps` this patch would cause a ~25% slowdown (and a 33% increase in allocations). Not too concerning, but since the purpose of this patch is to reduce String concat bootstrapping overhead it would be nice to not trade that for a localized regression. First improvement was to pack `TransformKey`s better: #8881 - which helps both variants, but doesn't really narrow the gap. We can improve further outside of `StringConcatFactory` to prepare for this RFE by reducing the number List-to-array and array-to-List conversions in `MethodHandles.dropArguments` (-280b/op). Another candidate is to turn `LambdaFormEditor` either into a static utility or it into `LamdaForm` proper (TBD if this is worthwhile; they're tightly coupled, and all state maintained by `LambdaFormEditor` lives in `LambdaForm`). - PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches
On Mon, 23 May 2022 20:03:05 GMT, Claes Redestad wrote: > When generating `MethodHandle`-based concatenation expressions in > `StringConcatFactory` we can reduce the number of classes generated at > runtime by creating small batches of prependers and mixers before binding > them into the root expression tree. > > Improvements on one-off tests are modest, while the improvement on > bootstrapping stress tests can be substantial > ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): > > | Build |# classes| Runtime | > | --- | - | --- | > | Baseline | 31119 | 2.942s | > | Patch | 16208 | 1.958s | > > An earlier version of this patch saw a regression in the > `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along > with the optimizations in #8881 and #8923 that is no longer the case, and > allocation pressure is down slightly compared to the baseline on such a > repeat-the-same-bootstrap stress test: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2170.039 ? 117.441 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3538.020 ?4.558B/op > > This patch: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2144.807 ? 162.685 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3184.943 ?4.495B/op LGTM - Marked as reviewed by jlaskey (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8855
RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches
When generating `MethodHandle`-based concatenation expressions in `StringConcatFactory` we can reduce the number of classes generated at runtime by creating small batches of prependers and mixers before binding them into the root expression tree. Improvements on one-off tests are modest, while the improvement on bootstrapping stress tests can be substantial ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): | Build |# classes| Runtime | | --- | - | --- | | Baseline | 31119 | 2.942s | | Patch | 16208 | 1.958s | An earlier version of this patch saw a regression in the `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along with the optimizations in #8881 and #8923 that is no longer the case, and allocation pressure is down slightly compared to the baseline on such a repeat-the-same-bootstrap stress test: Baseline: Benchmark Mode Cnt Score Error Units SCFB.makeConcatWithConstants avgt5 2170.039 ? 117.441 ns/op SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 3538.020 ?4.558B/op This patch: Benchmark Mode Cnt Score Error Units SCFB.makeConcatWithConstants avgt5 2144.807 ? 162.685 ns/op SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 3184.943 ?4.495B/op - Commit messages: - Revert change to HelloClasslist (doesn't affect generation) - Reduce allocation adding in mixers by extracting constant arg lists and caching double arg mixers - De-CHM the prepender and mixer caches - Reduce allocations: Extract constant argument lists, cache base form for two-arg prependers - Refactor, reduce allocations - Merge branch 'master' into scf_chunked - Improve chunking further - Fix mis-merged change - Experiment: Chunk prependers and mixers to reduce number of rebinds and generated LF classes Changes: https://git.openjdk.java.net/jdk/pull/8855/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8855=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287522 Stats: 390 lines in 2 files changed: 232 ins; 99 del; 59 mod Patch: https://git.openjdk.java.net/jdk/pull/8855.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8855/head:pull/8855 PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. The change looks good to me. I've checked the fix with our CI system and no `java.naming` failures have been discovered. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/8938
Re: RFR: 8284638: store skip buffers in InputStream Object [v15]
On Mon, 30 May 2022 05:29:01 GMT, Alan Bateman wrote: >> More practically. >> This PR has a noticeable negative effect - it increases the size of >> InputStream objects. Moreover, it increases the size of InputStream >> subclasses which has own skip() implementation and don't need this >> superclass modification. >> >> Let's look into InputStream subclasses. >> I've checked 51 InputStream from classlib. 30 of them either have their own >> skip() implementation or use super.skip() other than from InputStream. This >> PR is definitely harmful to these 30 classes. These 30 classes can be >> divided into the following categories: >> >> 1) Clean non-allocating skip() implementation (22 classes): >> - BufferedInputStream (java.io) >> - ByteArrayInputStream (java.io) >> - FileInputStream (java.io) >> - FilterInputStream (java.io) >> - PeekInputStream in ObjectInputStream (java.io) >> - BlockDataInputStream in ObjectInputStream (java.io) >> - PushbackInputStream (java.io) >> - FastInputStream in Manifest (java.util.jar) >> - ZipFileInputStream in ZipFile (java.util.zip) >> - CipherInputStream (javax.crypto) >> - MeteredStream (sun.net.www) >> - Anonymous in nullInputStream() in InputStream (java.io) >> - DataInputStream (java.io) >> - Anonymous in readTrailer() in GZIPInputStream (java.util.zip) >> - FtpInputStream in FtpURLConnection (sun.net.www.protocol.ftp) >> - JarURLInputStream in JarURLConnection (sun.net.www.protocol.jar) >> - PlainTextInputStream (sun.net.www.content.text) >> - PushbackInputStream (java.io) >> - TelnetInputStream (sun.net) >> - UnclosableInputStream in FileInputStreamPool (sun.security.provider) >> - MeteredStream (sun.net.www) >> - KeepAliveStream (sun.net.www.http) >> >> 2) Partially clean skip() implementation (1 class): >> - ChannelInputStream (sun.nio.ch) >>Note: clean skip impl when using SeekableByteChannel (most usages) >> otherwise super.skip() is used, and it may be easily rewritten using the >> existing internal buffer. >> >> 3) Unavoidable skip buffers (7 classes): >> - PipeInputStream in Process (java.lang) // per skip() invocation buffer >> byte[2048] >> - CheckedInputStream (java.util.zip) // per skip() invocation buffer >> byte[512] >> - InflaterInputStream (java.util.zip)// per skip() invocation buffer >> byte[512] >> - AppInputStream in SSLSocketImpl (sun.security.ssl) // per skip() >> invocation buffer byte[256] >> - DeflaterInputStream (java.util.zip) // cached skip buffer, byte[512], >> allocated on demand >> - ZipInputStream (java.util.zip) // preallocated skip buffer >> byte[512] >> - HttpInputStream in HttpURLConnection (sun.net.www.protocol.http) // >> cached skip buffer, byte[8096], allocated on demand >> >> It would be better to clean all skip implementations for the latter category >> and make it consistent. Now it's totally a mess. All possible strategies >> were implemented. >> >> Now let's consider classes which uses InputStream.skip() implementation (21 >> classes): >> >> 4) skip() is not implemented, when trivial implementation is possible (4 >> classes): >> - EmptyInputStream (sun.net.www.protocol.http) >> - NullInputStream in ProcessBuilder (java.lang) >> - ObjectInputStream (java.io) >> - extObjectInputStream (javax.crypto) >> >> 5) skip() is not implemented, when not-so-trivial implementation is possible >> (9 classes): >> - Anonymous in getInputStream() in NioSocketImpl (sun.nio.ch) >>Notes: temp direct buffer is used for reading, it's possible to implement >> skip over the direct buffer, save allocation + no copying from direct buffer >> to byte[] >> - Anonymous in newInputStream() in Channels (java.nio.channels) >>Notes: temp direct buffer is used for reading, it's possible to implement >> skip over the direct buffer, save allocation + no copying from direct buffer >> to byte[] >> - ChunkedInputStream (sun.net.www.http) >>Notes: skip() maybe implemented with the existing internal buffer >> - DecInputStream in Base64 (java.util) >> - ErrorStream in HttpURLConnection (sun.net.www.protocol.http) >>Notes: skip (but easily can be implemented with internal buffer + >> delegation to tail stream) >> - PipedInputStream (java.io) >>Notes: skip() may be implemented with the existing internal buffer >> - SequenceInputStream (java.io) >>Notes: skip() maybe implemented with delegation >> - SocketInputStream (sun.nio.ch) >>Notes: temp direct buffer is used for reading, it's possible to implement >> skip over the direct buffer, save allocation + no copying from direct buffer >> to byte[] >> - SocketInputStream in Socket (java.net) >>Notes: skip() maybe implemented with delegation >> >> And the last category: >> >> 6) skip() is not implemented, and skip buffer is unavoidable (8 classes): >> - VerifierStream in JarVerifier (java.util.jar) >> - DigestInputStream (java.security) >> - HttpCaptureInputStream
Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]
On Mon, 30 May 2022 00:10:50 GMT, Tim Prinzing wrote: >> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates >> a test module with some resources in it for the actual tests that occur at >> the native level. The native part was switched to c++ instead of c to make >> it easier to create helper objects that reduce the redundant code performing >> error checking. The two helper classes InstanceCall and StaticCall were >> placed in an include file called CallHelper.hpp. The main part of the tests >> are in exeNullCallerTest.cpp, and there is a separate function for each of >> the bug reports. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > problem with iostream on Windows, use printf Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8934
Re: RFR: 8284638: store skip buffers in InputStream Object [v15]
On Mon, 30 May 2022 05:29:01 GMT, Alan Bateman wrote: > However, I think your suggestion to change the no-arg read/write be > non-abstract is interesting as it's always a pain to have to implement that. @AlanBateman this need a csr IMO? - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8287497: Use String.contains() instead of String.indexOf() in java.naming
On Sun, 29 May 2022 14:00:20 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in java.naming still uses old approach with `String.indexOf` to > check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. _(not a reviewer)_ Provided `indexOf() != -1` and `indexOf() >= 0` are equivalent (which should indeed be the case, according to the Javadoc), the changes look good - PR: https://git.openjdk.java.net/jdk/pull/8938
Integrated: 8287363: null pointer should use NULL instead of 0
On Sat, 28 May 2022 03:31:30 GMT, Yasumasa Suenaga wrote: > We found using `0` as `NULL` in java_md_common.c . `0` is not a pointer, so > we should use `NULL` where we want to handle it. > > https://github.com/openjdk/jdk/pull/8646#discussion_r882294076 > > Also I found using `0` as NUL char in java_md_common.c , so I replaced it to > `\0` in this PR. This pull request has now been integrated. Changeset: a27ba1a3 Author:Yasumasa Suenaga URL: https://git.openjdk.java.net/jdk/commit/a27ba1a3db5f0b4eb75b6cca94f33398e7b695cc Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod 8287363: null pointer should use NULL instead of 0 Reviewed-by: kbarrett, stuefe, alanb - PR: https://git.openjdk.java.net/jdk/pull/8931
Integrated: 8287073: NPE from CgroupV2Subsystem.getInstance()
On Fri, 20 May 2022 08:34:58 GMT, Maxim Kartashev wrote: > Following the logic from the comment directly above the changed line, since > it doesn't matter which controller we pick, pick any available controller > instead of the one called "memory" specifically. This way we are guarded > against getting `null` as `anyController`, which is being immediately passed > down to `CgroupV2Subsystem.getInstance()` that is unprepared to accept `null` > values. > > It is also worth noting that the previous checks (such as that at line 89) > make sure that there exist at least one controller in the map. This pull request has now been integrated. Changeset: 744b822a Author:Maxim Kartashev Committer: Ioi Lam URL: https://git.openjdk.java.net/jdk/commit/744b822ab194a0f7ef4e7a4053be32c6a0889efc Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod 8287073: NPE from CgroupV2Subsystem.getInstance() Reviewed-by: vkempik, iklam - PR: https://git.openjdk.java.net/jdk/pull/8803
Re: RFR: 8287073: NPE from CgroupV2Subsystem.getInstance() [v3]
On Thu, 26 May 2022 16:04:17 GMT, Maxim Kartashev wrote: >> Following the logic from the comment directly above the changed line, since >> it doesn't matter which controller we pick, pick any available controller >> instead of the one called "memory" specifically. This way we are guarded >> against getting `null` as `anyController`, which is being immediately passed >> down to `CgroupV2Subsystem.getInstance()` that is unprepared to accept >> `null` values. >> >> It is also worth noting that the previous checks (such as that at line 89) >> make sure that there exist at least one controller in the map. > > Maxim Kartashev has updated the pull request incrementally with one > additional commit since the last revision: > > Made requested changes I tested the patch in our CI pipeline. All container tests passed. - PR: https://git.openjdk.java.net/jdk/pull/8803
