Re: RFR: 8330686: Fix typos in the DatabaseMetaData javadoc [v2]
On Tue, 23 Apr 2024 09:32:40 GMT, Jin Kwon wrote: >> Fix typos within the `DatabaseMetaData.java`. > > Jin Kwon has updated the pull request incrementally with one additional > commit since the last revision: > > [JDK-8330686] Update copyright year Thank you for the update. The changes look fine to me. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18860#pullrequestreview-2018924833
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v3]
> Prevent blocking due to a carrier thread not being released when > `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8330748: Add vthread1.join() in test - Changes: - all: https://git.openjdk.org/jdk/pull/18901/files - new: https://git.openjdk.org/jdk/pull/18901/files/57e4917b..1dd59b7b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18901&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18901&range=01-02 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18901.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18901/head:pull/18901 PR: https://git.openjdk.org/jdk/pull/18901
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 06:20:47 GMT, Alan Bateman wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Correct ID in test @bug tag
>
> test/jdk/java/io/ByteArrayOutputStream/WriteToReleasesCarrier.java line 78:
>
>> 76: }
>> 77: } finally {
>> 78: LockSupport.unpark(vthread1);
>
> It might be clearer if you add vthread1.join() after the unpark. It's not
> strictly needed here as scheduler::close will block until the carrier
> terminates so that will guarantee that the virtual thread has unmounted.
Added `vthread1.join()` after the unpark in
1dd59b7bf7aa2087845ad7806a5afbed2b5ea1b5.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1577076109
Re: RFR: 8327791: Optimization for new BigDecimal(String) [v17]
> The current BigDecimal(String) constructor calls String#toCharArray, which
> has a memory allocation.
>
>
> public BigDecimal(String val) {
> this(val.toCharArray(), 0, val.length()); // allocate char[]
> }
>
>
> When the length is greater than 18, create a char[]
>
>
> boolean isCompact = (len <= MAX_COMPACT_DIGITS); // 18
> if (!isCompact) {
> // ...
> } else {
> char[] coeff = new char[len]; // allocate char[]
> // ...
> }
>
>
> This PR eliminates the two memory allocations mentioned above, resulting in
> an approximate 60% increase in performance..
Shaojin Wen 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 21 additional commits since the
last revision:
- Merge remote-tracking branch 'upstream/master' into optim_dec_new
- use while instead for
- Update src/java.base/share/classes/java/math/BigDecimal.java
Co-authored-by: Claes Redestad
- bug fix for CharArraySequence#charAt
- bug fix for CharArraySequence
- fix benchmark
- one CharArraySequence
- restore comment
- easier to compare
- fix CharArraySequence
- ... and 11 more: https://git.openjdk.org/jdk/compare/886ef758...e516b580
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18177/files
- new: https://git.openjdk.org/jdk/pull/18177/files/17f0a736..e516b580
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18177&range=16
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18177&range=15-16
Stats: 562957 lines in 6174 files changed: 85928 ins; 137051 del; 339978 mod
Patch: https://git.openjdk.org/jdk/pull/18177.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18177/head:pull/18177
PR: https://git.openjdk.org/jdk/pull/18177
Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v24]
> Classes in the `java.lang.ref` package would benefit from an update to bring > the spec in line with how the VM already behaves. The changes would focus on > _happens-before_ edges at some key points during reference processing. > > A couple key things we want to be able to say are: > - `Reference.reachabilityFence(x)` _happens-before_ reference processing > occurs for 'x'. > - `Cleaner.register()` _happens-before_ the Cleaner thread runs the > registered cleaning action. > > This will bring Cleaner in line (or close) with the memory visibility > guarantees made for finalizers in [JLS > 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5): > _"There is a happens-before edge from the end of a constructor of an object > to the start of a finalizer (§12.6) for that object."_ Brent Christian has updated the pull request incrementally with one additional commit since the last revision: move reachability section before notification; update section header - Changes: - all: https://git.openjdk.org/jdk/pull/16644/files - new: https://git.openjdk.org/jdk/pull/16644/files/27d0c249..cc21d296 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16644&range=23 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16644&range=22-23 Stats: 67 lines in 1 file changed: 33 ins; 33 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16644.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16644/head:pull/16644 PR: https://git.openjdk.org/jdk/pull/16644
Integrated: 8329222: java.text.NumberFormat (and subclasses) spec updates
On Wed, 10 Apr 2024 20:16:50 GMT, Justin Lu wrote: > Please review this PR which is a large spec reformatting for > _java.text.NumberFormat_ and related subclasses, specifically DecimalFormat > and CompactNumberFormat. > > The motivation for this change was the difficulty of readability for these > classes. This PR consists of changes such as better separating the text into > sections with headers, advising to skip the pattern related sections if not > needed, and general wording improvements. > > To help with reviewing I have attached a > [specdiff](https://cr.openjdk.org/~jlu/JDK-8329222/java.base/java/text/package-summary.html) > as well as the built docs: > [nFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/NumberFormat.html), > > [dFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/DecimalFormat.html), > and > [cnFmt](https://cr.openjdk.org/~jlu/api/java.base/java/text/CompactNumberFormat.html); > > I will update the CSR once the wording is finalized. This pull request has now been integrated. Changeset: f60798a3 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/f60798a30e9a3e0b666fed5384b6ac92a8a283dd Stats: 598 lines in 3 files changed: 234 ins; 204 del; 160 mod 8329222: java.text.NumberFormat (and subclasses) spec updates Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/18731
Re: RFR: 8327247: C2 uses up to 2GB of RAM to compile complex string concat in extreme cases [v6]
On Thu, 18 Apr 2024 14:50:30 GMT, Claes Redestad wrote: >> This patch suggests a workaround to an issue with huge SCF MH expression >> trees taking excessive JIT compilation resources by reviving (part of) the >> simple bytecode-generating strategy that was originally available as an >> all-or-nothing strategy choice. >> >> Instead of reintroducing a binary strategy choice I propose a threshold >> parameter, controlled by >> `-Djava.lang.invoke.StringConcat.highArityThreshold=`: For expressions >> below or at this threshold there's no change, for expressions with an arity >> above it we use the `StringBuilder`-chain bytecode generator. >> >> There are a few trade-offs at play here which influence the choice of >> threshold. The simple high arity strategy will for example not see any reuse >> of LambdaForms but strictly always generate a class per indy callsite, which >> means we might end up with a higher total number of classes generated and >> loaded in applications if we set this value too low. It may also produce >> worse performance on average. On the other hand there is the observed >> increase in C2 resource usage as expressions grow unwieldy. On the other >> other hand high arity expressions are likely rare to begin with, with less >> opportunities for sharing than the more common low-arity expressions. >> >> I turned the submitted test case into a few JMH benchmarks and did some >> experiments with `-XX:CompileCommand=MemStat,StringConcat::concat,print`: >> >> Baseline strategy: >> 13 args: 6.3M >> 23 args: 18M >> 123 args: 868M >> >> `-Djava.lang.invoke.StringConcat.highArityThreshold=0`: >> 13 args: 2.11M >> 23 args: 3.67M >> 123 args: 4.75M >> >> For 123 args the memory overhead of the baseline strategy is 180x, but for >> 23 args we're down to a 5x memory overhead, and down to a 3x overhead for 13 >> args. Since the absolute overhead for 23 is borderline acceptable (+15Mb) >> I've conservatively chosen a threshold at arity 20. This keeps C2 resource >> pressure at a reasonable level (< 18M) while avoiding perturbing performance >> at the vast majority of call sites. >> >> I was asked to use the new class file API for mainline. There's a version of >> this patch implemented using ASM in 7c52a9f which might be a reasonable >> basis for a backport. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Minor SimpleStringBuilderStrategy:: overhead reduction src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1108: > 1106: DUMPER.dumpClass(innerClass.getName(), innerClass, > classBytes); > 1107: MethodHandle mh = hiddenLookup.findStatic(innerClass, > METHOD_NAME, args); > 1108: return mh; An alternative way to define a hidden class that will detect if the dumper is enabled and dump the classfile. Suggestion: Lookup hiddenLookup = lookup.makeHiddenClassDefiner(className, classBytes, Set.of(STRONG), DUMPER) .defineClassAsLookup(true); Class innerClass = hiddenLookup.lookupClass(); return hiddenLookup.findStatic(innerClass, METHOD_NAME, args); - PR Review Comment: https://git.openjdk.org/jdk/pull/18690#discussion_r1576880600
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v12]
On Tue, 23 Apr 2024 13:54:42 GMT, Evemose wrote:
>> **Subject**
>> Addition of Predicate-based `indexOf` and `lastIndexOf` methods to
>> `java.util.List`
>>
>> **Motivation**
>> The motivation behind this proposal is to enhance the functionality of the
>> `List` interface by providing a more flexible way to find the index of an
>> element. Currently, the `indexOf` and `lastIndexOf` methods only accept an
>> object as a parameter. This limits the flexibility of these methods as they
>> can only find the index of exact object matches.
>>
>> The proposed methods would accept a `Predicate` as a parameter, allowing
>> users to define a condition that the desired element must meet. This would
>> provide a more flexible and powerful way to find the index of an element in
>> a list.
>>
>> Here is a brief overview of the changes made in this pull request:
>>
>> 1. Added the `indexOf(Predicate filter)` method to the `List`
>> interface.
>> 2. Added the `lastIndexOf(Predicate filter)` method to the `List`
>> interface.
>> 3. Implemented these methods in all non-abstract classes that implement the
>> `List` interface.
>>
>> The changes have been thoroughly tested to ensure they work as expected and
>> do not introduce any regressions. The test cases cover a variety of
>> scenarios to ensure the robustness of the implementation.
>>
>> For example, consider the following test case:
>>
>> List list = new ArrayList<>();
>> list.add("Object one");
>> list.add("NotObject two");
>> list.add("NotObject three");
>>
>> int index1 = list.indexOf(s -> s.contains("ct t"));
>> System.out.println(index1); // Expected output: 1
>> int index2 = list.lastIndexOf(s -> s.startsWith("NotObject"));
>> System.out.println(index2); // Expected output: 2
>>
>>
>> Currently, to achieve the same result, we would have to use a more verbose
>> approach:
>>
>> int index1 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).contains("ct t"))
>> .findFirst()
>> .orElse(-1);
>> System.out.println(index1); // Output: 1
>> int index2 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).startsWith("NotObject"))
>> .reduce((first, second) -> second)
>> .orElse(-1);
>> System.out.println(index2); // Output: 2
>>
>>
>> I believe these additions would greatly enhance the functionality and
>> flexibility of the `List` interface, making it more powerful and
>> user-friendly. I look forward to your feedback and am open to making any
>> necessary changes bas...
>
> Evemose has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - Added Objects import to sun List
> - Replaced on-demand import in com.sunList
> - added non-null assertions
These can be `private` as they’re not used outside the `ArrayList` class nest:
src/java.base/share/classes/java/util/ArrayList.java line 324:
> 322: }
> 323:
> 324: int findIndexInRange(Predicate filter, int start, int
> end) {
Suggestion:
private int findIndexInRange(Predicate filter, int start, int
end) {
src/java.base/share/classes/java/util/ArrayList.java line 380:
> 378: }
> 379:
> 380: int findLastIndexInRange(Predicate filter, int start, int
> end) {
Suggestion:
private int findLastIndexInRange(Predicate filter, int start,
int end) {
-
PR Review: https://git.openjdk.org/jdk/pull/18639#pullrequestreview-2018246257
PR Review Comment: https://git.openjdk.org/jdk/pull/18639#discussion_r1576858232
PR Review Comment: https://git.openjdk.org/jdk/pull/18639#discussion_r1576858391
Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v7]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains 10 commits:
- Merge with upstream/master
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- Merge with upstream/master
- update test
- improve handling of ignorable doc comments
suppress warning when building test code
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- call `saveDanglingDocComments` for local variable declarations
- adjust call for `saveDanglingDocComments` for enum members
- JDK-8303689: javac -Xlint could/should report on "dangling" doc comments
-
Changes: https://git.openjdk.org/jdk/pull/18527/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=06
Stats: 485 lines in 59 files changed: 387 ins; 3 del; 95 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v2]
On Tue, 2 Apr 2024 19:19:59 GMT, Volodymyr Paprotski wrote:
>> Performance. Before:
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt ScoreError Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt3 6443.934 ± 6.491 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt3 6152.979 ± 4.954 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt3 1895.410 ± 36.979 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt3 1878.955 ± 45.487 ops/s
>> Benchmark(algorithm)
>> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units
>> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1357.810 ± 26.584 ops/s
>> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1352.119 ± 23.547 ops/s
>> Benchmark (isMontBench) Mode Cnt Score
>> Error Units
>> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ±
>> 10.970 ops/s
>>
>> Performance, no intrinsic:
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt Score Error Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt3 6529.839 ± 42.420 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt3 6199.747 ± 133.566 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt3 1973.676 ± 54.071 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt3 1932.127 ± 35.920 ops/s
>> Benchmark(algorithm)
>> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units
>> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1355.788 ± 29.858 ops/s
>> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1346.523 ± 28.722 ops/s
>> Benchmark (isMontBench) Mode Cnt Score
>> Error Units
>> PolynomialP256Bench.benchMultiply true thrpt3 1919.57...
>
> Volodymyr Paprotski has updated the pull request incrementally with one
> additional commit since the last revision:
>
> remove use of jdk.crypto.ec
src/java.base/share/classes/sun/security/ec/ECOperations.java line 308:
> 306:
> 307: /*
> 308: * public Point addition. Used by ECDSAOperations
Was the old description not applicable anymore? It would be nice to improve
on the existing description that shortening it.
src/java.base/share/classes/sun/security/ec/ECOperations.java line 321:
> 319: ECOperations ops = this;
> 320: if (this.montgomeryOps != null) {
> 321: assert p.getField() instanceof IntegerMontgomeryFieldModuloP;
This should throw a ProviderException, I believe this would throw an
AssertionException
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1556740469
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1558824543
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v3]
On Mon, 15 Apr 2024 22:12:30 GMT, Volodymyr Paprotski wrote:
>> Performance. Before:
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt ScoreError Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt3 6443.934 ± 6.491 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt3 6152.979 ± 4.954 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt3 1895.410 ± 36.979 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt3 1878.955 ± 45.487 ops/s
>> Benchmark(algorithm)
>> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units
>> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1357.810 ± 26.584 ops/s
>> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1352.119 ± 23.547 ops/s
>> Benchmark (isMontBench) Mode Cnt Score
>> Error Units
>> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ±
>> 10.970 ops/s
>>
>> Performance, no intrinsic:
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt Score Error Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt3 6529.839 ± 42.420 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt3 6199.747 ± 133.566 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt3 1973.676 ± 54.071 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt3 1932.127 ± 35.920 ops/s
>> Benchmark(algorithm)
>> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units
>> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1355.788 ± 29.858 ops/s
>> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1346.523 ± 28.722 ops/s
>> Benchmark (isMontBench) Mode Cnt Score
>> Error Units
>> PolynomialP256Bench.benchMultiply true thrpt3 1919.57...
>
> Volodymyr Paprotski has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Comments from Jatin and Tony
src/java.base/share/classes/sun/security/ec/ECOperations.java line 204:
> 202: * @return the product
> 203: */
> 204: public MutablePoint multiply(AffinePoint affineP, byte[] s) {
It seems like there could be some combining of both `multiply()`. If
`multiply(AffinePoint, ...)` is called, it can call `DefaultMultiplier` with
the `affineP`, but internally call the other `multiply(ECPoint, ...)` for the
other situations. I'd rather not have two methods doing most of the same code,
but different methods.
src/java.base/share/classes/sun/security/ec/ECOperations.java line 467:
> 465: sealed static abstract class SmallWindowMultiplier implements
> PointMultiplier
> 466: permits DefaultMultiplier, DefaultMontgomeryMultiplier {
> 467: private final AffinePoint affineP;
I don't think `affineP` needs to be a class variable anymore. It's only used
in the constructor
src/java.base/share/classes/sun/security/ec/ECOperations.java line 592:
> 590: }
> 591:
> 592: private final ProjectivePoint.Immutable[][] points;
Can you define this at the top please.
src/java.base/share/classes/sun/security/ec/ECOperations.java line 668:
> 666: }
> 667:
> 668: private final BigInteger[] base;
Can you define this at the top. You use it in the constructor but it's defined
later on.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1576821201
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1575499019
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1575495263
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1575491814
Re: RFR: 8328481: Implement Module Imports [v12]
> This is an implementation of JEP JDK-8315129: Module Import Declarations > (Preview). Please see the JEP for details: > https://bugs.openjdk.org/browse/JDK-8315129 > > It is mostly straightforward - the module imports are parsed, and then > expanded to import-on-demand in `TypeEnter`. > There is a few notable aspects, however: > - the AST node for import (`JCImport`) is holding the imported element as a > field access, because so far, the imported element always had to have a '.' > (even for import-on-demand). But for module imports, it is permissible to > import from a module whose name does not have a dot (`import module m;`). The > use of field access for ordinary import seems very useful, so I preferred to > keep that, and created a new internal-only AST node for module imports. There > is still only one public API AST node/interface, so this is purely an > implementation choice. > - JShell now supports module imports as well; and the default, implicit, > script is changed to use it to import all of `java.base` if preview is > enabled. It is expected that the default would be changed if/when the module > imports feature is finalized. Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision: Fixing Imports test on Windows. - Changes: - all: https://git.openjdk.org/jdk/pull/18614/files - new: https://git.openjdk.org/jdk/pull/18614/files/cbc363ab..1a621447 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18614&range=11 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18614&range=10-11 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18614.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18614/head:pull/18614 PR: https://git.openjdk.org/jdk/pull/18614
Re: RFR: 8327247: C2 uses up to 2GB of RAM to compile complex string concat in extreme cases [v6]
On Thu, 18 Apr 2024 14:50:30 GMT, Claes Redestad wrote: >> This patch suggests a workaround to an issue with huge SCF MH expression >> trees taking excessive JIT compilation resources by reviving (part of) the >> simple bytecode-generating strategy that was originally available as an >> all-or-nothing strategy choice. >> >> Instead of reintroducing a binary strategy choice I propose a threshold >> parameter, controlled by >> `-Djava.lang.invoke.StringConcat.highArityThreshold=`: For expressions >> below or at this threshold there's no change, for expressions with an arity >> above it we use the `StringBuilder`-chain bytecode generator. >> >> There are a few trade-offs at play here which influence the choice of >> threshold. The simple high arity strategy will for example not see any reuse >> of LambdaForms but strictly always generate a class per indy callsite, which >> means we might end up with a higher total number of classes generated and >> loaded in applications if we set this value too low. It may also produce >> worse performance on average. On the other hand there is the observed >> increase in C2 resource usage as expressions grow unwieldy. On the other >> other hand high arity expressions are likely rare to begin with, with less >> opportunities for sharing than the more common low-arity expressions. >> >> I turned the submitted test case into a few JMH benchmarks and did some >> experiments with `-XX:CompileCommand=MemStat,StringConcat::concat,print`: >> >> Baseline strategy: >> 13 args: 6.3M >> 23 args: 18M >> 123 args: 868M >> >> `-Djava.lang.invoke.StringConcat.highArityThreshold=0`: >> 13 args: 2.11M >> 23 args: 3.67M >> 123 args: 4.75M >> >> For 123 args the memory overhead of the baseline strategy is 180x, but for >> 23 args we're down to a 5x memory overhead, and down to a 3x overhead for 13 >> args. Since the absolute overhead for 23 is borderline acceptable (+15Mb) >> I've conservatively chosen a threshold at arity 20. This keeps C2 resource >> pressure at a reasonable level (< 18M) while avoiding perturbing performance >> at the vast majority of call sites. >> >> I was asked to use the new class file API for mainline. There's a version of >> this patch implemented using ASM in 7c52a9f which might be a reasonable >> basis for a backport. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Minor SimpleStringBuilderStrategy:: overhead reduction I really wish this change was not done with ClassFile API, but with a simple bundled ASM, so it would be easily backportable, if we decide to. It does not look like CFA buys us a lot here readability/complexity wise: https://github.com/openjdk/jdk/pull/18690/commits/d99b1596c5ca57b110c1db88be430c6c54c3d599 - PR Comment: https://git.openjdk.org/jdk/pull/18690#issuecomment-2073284920
Re: RFR: 8330542: Add two sample configuration files in preparation for a more secure by default configuration [v4]
On Tue, 23 Apr 2024 18:57:44 GMT, Sean Mullan wrote: > A few other comments/questions: > > Does this need a CSR since you are adding new property files? Not sure it does, but Joe will follow up with Joe Darcy > > Are there any tests to ensure the property files are working correctly? There are tests that Joe added as part of the JDK 22 work for custom config files > > Also, how does one try out these property files? Is there a system property > that needs to be set? Can you add more details to the RN on that? java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-compat.properties The property was added in JDK 22 see: https://docs.oracle.com/en/java/javase/22/docs/api/java.xml/module-summary.html#Conf_CF_SP - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2073248701
Re: RFR: 8330542: Add two sample configuration files in preparation for a more secure by default configuration [v4]
On Fri, 19 Apr 2024 21:55:09 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > changes description of jaxp-compat and jaxp-strict after discussing with > Lance Marked as reviewed by lancea (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18831#pullrequestreview-2018094529
Re: RFR: 8330542: Add two sample configuration files in preparation for a more secure by default configuration [v4]
On Fri, 19 Apr 2024 21:55:09 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > changes description of jaxp-compat and jaxp-strict after discussing with > Lance A few other comments/questions: Does this need a CSR since you are adding new property files? Are there any tests to ensure the property files are working correctly? Also, how does one try out these property files? Is there a system property that needs to be set? Can you add more details to the RN on that? - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2073203196
Re: RFR: 8330542: Add two sample configuration files in preparation for a more secure by default configuration [v4]
On Fri, 19 Apr 2024 21:55:09 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > changes description of jaxp-compat and jaxp-strict after discussing with > Lance It might just be me, but the word "sample" is too related to programming examples that it makes this feel like something that users may not take seriously. But I think this is something that you really want users to take seriously and try out. Perhaps just drop the word "sample"? - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2073191426
Re: RFR: 8329581: Java launcher no longer prints a stack trace [v4]
On Tue, 23 Apr 2024 18:21:42 GMT, Sonia Zaldana Calles
wrote:
>> Hi folks,
>>
>> This PR aims to fix
>> [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581).
>>
>> I think the regression got introduced in
>> [JDK-8315458](https://bugs.openjdk.org/browse/JDK-8315458).
>>
>> In the issue linked above,
>> [LauncherHelper#getMainType](https://github.com/openjdk/jdk/pull/16461/files#diff-108a3a3e3c2d108c8c7f19ea498f641413b7c9239ecd2975a6c27d904c2ba226)
>> got removed to simplify launcher code.
>>
>> Previously, we used ```getMainType``` to do the appropriate main method
>> invocation in ```JavaMain```. However, we currently attempt to do all types
>> of main method invocations at the same time
>> [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjli/java.c#L623).
>>
>>
>> Note how all of these invocations clear the exception reported with
>> [CHECK_EXCEPTION_FAIL](https://github.com/openjdk/jdk/blob/140f56718bbbfc31bb0c39255c68568fad285a1f/src/java.base/share/native/libjli/java.c#L390).
>>
>>
>> Therefore, if a legitimate exception comes up during one of these
>> invocations, it does not get reported.
>>
>> I propose reintroducing ```LauncherHelper#getMainType``` but I'm looking
>> forward to your suggestions.
>>
>> Cheers,
>> Sonia
>
> Sonia Zaldana Calles has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Changes based on feedback
I missed that `NULL_CHECK0` prints that JNI error, sorry. We should avoid
using it then. What do you think having a check macro to return 0 if
exception occurred or the given object is null? No exception cleared as it
will be described in JavaMain.
#define CHECK_EXCEPTION_NULL_FAIL(obj) \
do { \
if ((*env)->ExceptionOccurred(env)) { \
return 0; \
} else if (obj == NULL) { \
return 0; \
} \
} while (JNI_FALSE)
-
PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2073184282
Integrated: 8330178: Clean up non-standard use of /** comments in `java.base`
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote: > Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or import statements > * Misplaced doc comments after the annotations for a declaration > * Dangling `/**` comments relating to a group of declarations, separate from > the doc comments for each of those declarations > * Use of `/**` for the legal header at or near the top of the file > > In one case, two doc comments before a declaration were merged, which fixes a > bug/omission in the documented serialized form. This pull request has now been integrated. Changeset: 9cc163a9 Author:Jonathan Gibbons URL: https://git.openjdk.org/jdk/commit/9cc163a999eb8e9597d45b095b642c25071043bd Stats: 134 lines in 23 files changed: 50 ins; 56 del; 28 mod 8330178: Clean up non-standard use of /** comments in `java.base` Reviewed-by: darcy, iris, dfuchs, aivanov, naoto - PR: https://git.openjdk.org/jdk/pull/18846
Re: RFR: 8329581: Java launcher no longer prints a stack trace
On Tue, 23 Apr 2024 18:21:30 GMT, Sonia Zaldana Calles wrote: > The JNI error message didn’t previously get reported before the regression > was introduced, so I just wanted to make sure we were okay with this. I think such errors have a very high potential to confuse the hell out of the poor ops people looking for binary corruptions and stuff when it’s “only” a regular Java level error condition. So I think it should not talk about JNI in this case (even when the rest of the exception makes it obvious) - PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2073171918
Re: RFR: 8329581: Java launcher no longer prints a stack trace
On Fri, 19 Apr 2024 17:04:30 GMT, Mandy Chung wrote: >> @lahodaj >> >>> I would suggest to take the test from 18753 though - doing a change like >>> this without a test may lead to hard-to-find regressions in the future. >>> (Note the current test should guard against both JDK-8329420 and >>> JDK-8329581.) >> >> Agreed. I’ll add the test case if this PR proceeds (see my comment above). >> >>> as Mandy points out, `LaucherHelper` already reads/has variables for >>> "is-static" and "no-arguments" in `validateMainMethod`, so it should be >>> possible to just use that values; >> >> Just to clarify, this would still mean converting “isStatic” and “noArgs” >> from local variables to fields so I am able to read them on the C side of >> things. Did I understand this correctly? >> >>> also as Mandy points out, we can probably get rid of `CHECK_EXCEPTION_FAIL` >>> and `CHECK_EXCEPTION_NULL_FAIL`, and use the `..._LEAVE` variants, no? (The >>> `..._FAIL` variants where needed so that the launcher could continue with >>> the next variant, but since we now only call the correct variant, we can >>> just stop if something goes wrong?) >> >> Agreed, I’ve updated that on my side of things. > >> Just to clarify, this would still mean converting “isStatic” and “noArgs” >> from local variables to fields so I am able to read them on the C side of >> things. Did I understand this correctly? > > I'm okay with adding static boolean fields and read by the native code and > the name can be explicit like `isStaticMain` and `mainWithNoArg`. Hi @mlchung, thanks for the feedback! I’ve pushed the updates. Just a question about ```NULL_CHECK0```. ```NULL_CHECK0``` reports the error message and then the exception is described in ```CHECK_EXCEPTION_LEAVE```. This results in a stack trace that looks like this: Error: A JNI error has occurred, please check your installation and try again Exception in thread "main" java.lang.NoClassDefFoundError: DemoSonia$SomeDependency at DemoSonia.(DemoSonia.java:3) Caused by: java.lang.ClassNotFoundException: DemoSonia$SomeDependency at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641) at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:528) ... 1 more The JNI error message didn’t previously get reported before the regression was introduced, so I just wanted to make sure we were okay with this. Looking forward to your comments! - PR Comment: https://git.openjdk.org/jdk/pull/18786#issuecomment-2073092870
Re: RFR: 8329581: Java launcher no longer prints a stack trace [v4]
> Hi folks, > > This PR aims to fix > [JDK-8329581](https://bugs.openjdk.org/browse/JDK-8329581). > > I think the regression got introduced in > [JDK-8315458](https://bugs.openjdk.org/browse/JDK-8315458). > > In the issue linked above, > [LauncherHelper#getMainType](https://github.com/openjdk/jdk/pull/16461/files#diff-108a3a3e3c2d108c8c7f19ea498f641413b7c9239ecd2975a6c27d904c2ba226) > got removed to simplify launcher code. > > Previously, we used ```getMainType``` to do the appropriate main method > invocation in ```JavaMain```. However, we currently attempt to do all types > of main method invocations at the same time > [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjli/java.c#L623). > > > Note how all of these invocations clear the exception reported with > [CHECK_EXCEPTION_FAIL](https://github.com/openjdk/jdk/blob/140f56718bbbfc31bb0c39255c68568fad285a1f/src/java.base/share/native/libjli/java.c#L390). > > > Therefore, if a legitimate exception comes up during one of these > invocations, it does not get reported. > > I propose reintroducing ```LauncherHelper#getMainType``` but I'm looking > forward to your suggestions. > > Cheers, > Sonia Sonia Zaldana Calles has updated the pull request incrementally with one additional commit since the last revision: Changes based on feedback - Changes: - all: https://git.openjdk.org/jdk/pull/18786/files - new: https://git.openjdk.org/jdk/pull/18786/files/79c70343..66942238 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18786&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18786&range=02-03 Stats: 65 lines in 2 files changed: 8 ins; 7 del; 50 mod Patch: https://git.openjdk.org/jdk/pull/18786.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18786/head:pull/18786 PR: https://git.openjdk.org/jdk/pull/18786
Re: RFR: 8327247: C2 uses up to 2GB of RAM to compile complex string concat in extreme cases [v6]
On Mon, 22 Apr 2024 08:59:33 GMT, Claes Redestad wrote: > @mlchung @asotona: Alan asked me to use the ClassFile API here. As it's only > used in what's effectively a slow path it shouldn't be blocked by the startup > investigations we're doing there, right? I agree that this should not be blocked by the startup investigation for ClassFile API. It only affects high arity expressions. - PR Comment: https://git.openjdk.org/jdk/pull/18690#issuecomment-2073085437
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 17:35:42 GMT, Jason Mehrens wrote: >> A good question. The buf/count fields are protected so the subclass has >> direct access to the bytes. So while it could Arrays.copy the bytes, it >> doesn't help with a buggy subclass that is changing bytes without >> synchronization. > > I was thinking more of a subclass that counted invocations to public methods > or metering which would cause subclass to double the counts when calling via > virtual thread. So do we think it better not to invoke `toByteArray` here? - PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1576686390
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 16:08:13 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/io/ByteArrayOutputStream.java line 164:
>>
>>> 162: public void writeTo(OutputStream out) throws IOException {
>>> 163: if (Thread.currentThread().isVirtual()) {
>>> 164: out.write(toByteArray());
>>
>> Would it be better to avoid calling a public method `toByteArray` encase
>> subclass is overriding it?
>
> A good question. The buf/count fields are protected so the subclass has
> direct access to the bytes. So while it could Arrays.copy the bytes, it
> doesn't help with a buggy subclass that is changing bytes while
> synchronization.
I was thinking more of a subclass that counted invocations to public methods or
metering which would cause subclass to double the counts when calling via
virtual thread.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1576637177
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O [v2]
On Tue, 23 Apr 2024 08:09:20 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/io/FileInputStream.java line 211:
>>
>>> 209: * @param name the name of the file
>>> 210: */
>>> 211: private void open(String name) throws FileNotFoundException {
>>
>> If method such as this is private, and only delegates to the 0-suffixed
>> native method, would't it be better to just call the 0-suffixed method
>> directly?
>
> Historically these native methods were wrapped in order to support
> instrumentation, I didn't want to touch in this PR.
And presumably all these 0-suffixed methods will eventually be replaced with
FFM calls anyway.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18598#discussion_r1576561165
Integrated: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O
On Wed, 3 Apr 2024 10:04:36 GMT, Alan Bateman wrote: > This change drops the adjustments to the virtual thread scheduler's target > parallelism when virtual threads do file operations on files that are opened > for buffered I/O. These operations are usually just too short to have any > benefit and may have a negative benefit when reading/writing a small number > of bytes. There is no change for read/write operations on files opened for > direct I/O or when writing to files that are opened with options for > synchronized I/O file integrity (O_SYNC/O_DSYNC and equivalents). Sergery > Kuksenko is polishing benchmarks that includes this area, this is for a > future PR. > > In addition, the blocker mechanism is updated to handle reentrancy (as can > happen if debugging code is added to ForkJoinPool) and preemption when > compensating (as can happen when substituting a heap buffer with a direct > buffer in some I/O operations). This part is a pre-requisite to the changes > to better support object monitor there are more places where preemption is > possible and this quickly leads to unbalanced begin/end. > > The changes have been baking in loom repo for some time. This pull request has now been integrated. Changeset: 412e306d Author:Alan Bateman URL: https://git.openjdk.org/jdk/commit/412e306d81209c05f55aee7663f7abb80286e361 Stats: 1084 lines in 28 files changed: 215 ins; 624 del; 245 mod 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O Reviewed-by: bpb, jpai - PR: https://git.openjdk.org/jdk/pull/18598
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 11:16:01 GMT, Jason Mehrens wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Correct ID in test @bug tag
>
> src/java.base/share/classes/java/io/ByteArrayOutputStream.java line 164:
>
>> 162: public void writeTo(OutputStream out) throws IOException {
>> 163: if (Thread.currentThread().isVirtual()) {
>> 164: out.write(toByteArray());
>
> Would it be better to avoid calling a public method `toByteArray` encase
> subclass is overriding it?
A good question. The buf/count fields are protected so the subclass has direct
access to the bytes. So while it could Arrays.copy the bytes, it doesn't help
with a buggy subclass that is changing bytes while synchronization.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1576523112
Re: RFR: 8327858: Improve spliterator and forEach for single-element immutable collections [v2]
On Tue, 23 Apr 2024 13:13:03 GMT, Per Minborg wrote: > Do we need additional tests or are these modifications already covered by the > existing tests? Thanks for the note, upon review it seems the default method overrides aren't covered by existing Collection tests. I should add them to ensure the single/double element List/Set are tested. - PR Comment: https://git.openjdk.org/jdk/pull/15834#issuecomment-2072824367
Re: RFR: 7036144: GZIPInputStream readTrailer uses faulty available() test for end-of-stream [v9]
On Tue, 23 Apr 2024 12:49:15 GMT, Jaikiran Pai wrote: >> Archie Cobbs 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: >> >> - Merge branch 'master' into JDK-7036144 >> - Back-out Javadoc addition; to be added in a separate issue. >> - Document the handling of concatenated streams. >> - Merge branch 'master' into JDK-7036144 >> - Merge branch 'master' into JDK-7036144 >> - Merge branch 'master' into JDK-7036144 >> - Address third round of review comments. >> - Address second round of review comments. >> - Address review comments. >> - Fix bug in GZIPInputStream when underlying available() returns short. > > Hello Archie, we forgot to create a release note for this one (there's still > time). Would you be willing to create one, following the instructions here > https://openjdk.org/guide/#release-notes? If you need any help, let us know. > One of us will review that release note before you can Resolve it to > Delivered. Hi @jaikiran, No problem - please see [JDK-8330995](https://bugs.openjdk.org/browse/JDK-8330995) and let me know if anything else is needed. - PR Comment: https://git.openjdk.org/jdk/pull/17113#issuecomment-2072563144
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 14:25:22 GMT, Magnus Ihse Bursie wrote: > There's a huge amount of changes for just hsdis... You might have to separate > out the infrastructure changes that seem to amount to most of the changes > here. > > This is going to take me a while to get through. Sorry, it's still a WIP, and I believe something broke and scattered changes from one of my other local branches into this current changeset - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2072483937
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust output - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/8b7a9d58..4d998244 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18699&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18699&range=03-04 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output I adjusted the output ot 'JRE path' . > The GetJREPath function has no business looking for jre/lib as that location > does not exist since JDK 9. Is the at/below `/* Does the app ship a private JRE in /jre directory? */` part meant? This looks indeed obsolete . - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072472623
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 13:56:32 GMT, Julian Waters wrote: > WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. There's a huge amount of changes for just hsdis... You might have to separate out the infrastructure changes that seem to amount to most of the changes here. This is going to take me a while to get through. - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2072458273
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Mon, 22 Apr 2024 11:57:19 GMT, Alan Bateman wrote: >> Hi, any additional comments / reviews ? >> Thanks Matthias > >> Hi, any additional comments / reviews ? Thanks Matthias > > It still looks like left over trace messages from a debugging session, need > to think about about what tracing make sense here. > @AlanBateman If this is not acceptable to add, I must once again iterate my > question: What is the use case for the tracing functionality then? The tracing was mostly for the options and timing when it was originally added. I don't object to expanding the tracing. The issue is that the proposed additions are inconsistent with the existing trace messages, e.g. GetJREPath's existing trace messages use "JRE path", the proposed messages switch to "JRE location". BTW: the vestiges of the "JRE" should really be removed. The GetJREPath function has no business looking for jre/lib as that location does not exist since JDK 9. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072415065
RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
WIP This changeset contains hsdis for Windows/gcc Port. It supports both the binutils and capstone backends, though the LLVM backend is left out due to compatibility issues encountered during the build. Currently, which gcc distributions are supported is still to be clarified, as several, ranging from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, the build system hack in place at the moment to compile the binutils backend on Windows is still left in place, for now. - Commit messages: - Implementation of hsdis for 8288293: Windows/gcc Port Changes: https://git.openjdk.org/jdk/pull/18915/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18915&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8330988 Stats: 347 lines in 28 files changed: 161 ins; 22 del; 164 mod Patch: https://git.openjdk.org/jdk/pull/18915.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18915/head:pull/18915 PR: https://git.openjdk.org/jdk/pull/18915
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v12]
On Sat, 20 Apr 2024 23:11:48 GMT, Chen Liang wrote:
>> Evemose has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - Added Objects import to sun List
>> - Replaced on-demand import in com.sunList
>> - added non-null assertions
>
> src/java.base/share/classes/java/util/LinkedList.java line 1507:
>
>> 1505:
>> 1506: public int findLastIndex(Predicate filter) {
>> 1507: return rlist.findLastIndex(filter);
>
> Hmm, this view is supposed to be reversed... so last and first need a swap.
> This is a bug in the current codebase, a good bug to fix indeed.
Ignore my fault, it's just delegating methods to 2 different, already reversed
views...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18639#discussion_r1573458008
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v12]
On Tue, 23 Apr 2024 13:51:54 GMT, Evemose wrote:
>> **Subject**
>> Addition of Predicate-based `indexOf` and `lastIndexOf` methods to
>> `java.util.List`
>>
>> **Motivation**
>> The motivation behind this proposal is to enhance the functionality of the
>> `List` interface by providing a more flexible way to find the index of an
>> element. Currently, the `indexOf` and `lastIndexOf` methods only accept an
>> object as a parameter. This limits the flexibility of these methods as they
>> can only find the index of exact object matches.
>>
>> The proposed methods would accept a `Predicate` as a parameter, allowing
>> users to define a condition that the desired element must meet. This would
>> provide a more flexible and powerful way to find the index of an element in
>> a list.
>>
>> Here is a brief overview of the changes made in this pull request:
>>
>> 1. Added the `indexOf(Predicate filter)` method to the `List`
>> interface.
>> 2. Added the `lastIndexOf(Predicate filter)` method to the `List`
>> interface.
>> 3. Implemented these methods in all non-abstract classes that implement the
>> `List` interface.
>>
>> The changes have been thoroughly tested to ensure they work as expected and
>> do not introduce any regressions. The test cases cover a variety of
>> scenarios to ensure the robustness of the implementation.
>>
>> For example, consider the following test case:
>>
>> List list = new ArrayList<>();
>> list.add("Object one");
>> list.add("NotObject two");
>> list.add("NotObject three");
>>
>> int index1 = list.indexOf(s -> s.contains("ct t"));
>> System.out.println(index1); // Expected output: 1
>> int index2 = list.lastIndexOf(s -> s.startsWith("NotObject"));
>> System.out.println(index2); // Expected output: 2
>>
>>
>> Currently, to achieve the same result, we would have to use a more verbose
>> approach:
>>
>> int index1 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).contains("ct t"))
>> .findFirst()
>> .orElse(-1);
>> System.out.println(index1); // Output: 1
>> int index2 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).startsWith("NotObject"))
>> .reduce((first, second) -> second)
>> .orElse(-1);
>> System.out.println(index2); // Output: 2
>>
>>
>> I believe these additions would greatly enhance the functionality and
>> flexibility of the `List` interface, making it more powerful and
>> user-friendly. I look forward to your feedback and am open to making any
>> necessary changes bas...
>
> Evemose has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - Added Objects import to sun List
> - Replaced on-demand import in com.sunList
> - added non-null assertions
src/java.base/share/classes/java/util/LinkedList.java line 1507:
> 1505:
> 1506: public int findLastIndex(Predicate filter) {
> 1507: return rlist.findLastIndex(filter);
Hmm, this view is supposed to be reversed... so last and first need a swap.
This is a bug in the current codebase, a good bug to fix indeed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18639#discussion_r1573456708
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v11]
On Fri, 19 Apr 2024 22:27:15 GMT, xxDark wrote:
>>> I noticed that most (if not all) methods don't ensure non-nullability of
>>> `filter` so NPE would only be thrown if the list is not empty.
>>
>> Yeah, thats true. not sure if it has to throw NPE even if list is emply
>
>> > I noticed that most (if not all) methods don't ensure non-nullability of
>> > `filter` so NPE would only be thrown if the list is not empty.
>>
>> Yeah, thats true. not sure if it has to throw NPE even if list is emply
>
> Yes, it does. If it shouldn't, then why isn't code in the java.util.List is
> like this:
>
> default int findIndex(Predicate filter) {
> ListIterator iterator = listIterator();
> if (!iterator.hasNext()) return -1;
> Objects.requireNonNull(filter);
> int index = 0;
> do {
> if (filter.test(iterator.next()))
> return index;
> index++;
> } while (iterator.hasNext());
> return -1;
> }
>
> Also see methods like `removveIf`. All methods should do checks even if they
> essentially do nothing.
@xxDark Added null asserions everywhere except sublists that delegate execution
to other lists, except for SynchroziedLsit so it doesnt acquire monitor with
invalid predicate. I think this would be the best choice
-
PR Comment: https://git.openjdk.org/jdk/pull/18639#issuecomment-2067762652
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v2]
On Fri, 19 Apr 2024 12:10:34 GMT, Chen Liang wrote: >> @minborg Sorry to bother you with this kind of question, but i cant manage >> to find where exactly and how I can file CSR review request. Could you help >> me out? > > @Evemose Please revert formatting changes in existing classes. Unfortunately > there's no single code style across all of JDK, but you shouldn't try to > format code/methods that's not touched by your new method addition. @liach Seems like discussion in mails has came to some conclusion (or people just arent interested in it anymore). Therefore, i think its time to file for csr. Here is a draft request text, but Im not sure how to fill in some fields: Summary: Add findIndex(Predicate) and findLastIndex(Predicate) to List Problem: Modification of java.utilList interface by adding findIndex(Predicate) and findLastIndex(Predicate) methods might introduce behavioural or byteecode incompatibilities Solution: // dont know what to write here tbh Specification: // do i have to paste changed code here, or provide links, or what? Alternatives: syntax line indexOf(Predicate) and lastIndexOf(Predicate) was concidered, but discarded due to behavioural incompatibilities with invokation indexOf(null) - PR Comment: https://git.openjdk.org/jdk/pull/18639#issuecomment-2072079213
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v11]
On Fri, 19 Apr 2024 15:49:11 GMT, Evemose wrote:
>> **Subject**
>> Addition of Predicate-based `indexOf` and `lastIndexOf` methods to
>> `java.util.List`
>>
>> **Motivation**
>> The motivation behind this proposal is to enhance the functionality of the
>> `List` interface by providing a more flexible way to find the index of an
>> element. Currently, the `indexOf` and `lastIndexOf` methods only accept an
>> object as a parameter. This limits the flexibility of these methods as they
>> can only find the index of exact object matches.
>>
>> The proposed methods would accept a `Predicate` as a parameter, allowing
>> users to define a condition that the desired element must meet. This would
>> provide a more flexible and powerful way to find the index of an element in
>> a list.
>>
>> Here is a brief overview of the changes made in this pull request:
>>
>> 1. Added the `indexOf(Predicate filter)` method to the `List`
>> interface.
>> 2. Added the `lastIndexOf(Predicate filter)` method to the `List`
>> interface.
>> 3. Implemented these methods in all non-abstract classes that implement the
>> `List` interface.
>>
>> The changes have been thoroughly tested to ensure they work as expected and
>> do not introduce any regressions. The test cases cover a variety of
>> scenarios to ensure the robustness of the implementation.
>>
>> For example, consider the following test case:
>>
>> List list = new ArrayList<>();
>> list.add("Object one");
>> list.add("NotObject two");
>> list.add("NotObject three");
>>
>> int index1 = list.indexOf(s -> s.contains("ct t"));
>> System.out.println(index1); // Expected output: 1
>> int index2 = list.lastIndexOf(s -> s.startsWith("NotObject"));
>> System.out.println(index2); // Expected output: 2
>>
>>
>> Currently, to achieve the same result, we would have to use a more verbose
>> approach:
>>
>> int index1 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).contains("ct t"))
>> .findFirst()
>> .orElse(-1);
>> System.out.println(index1); // Output: 1
>> int index2 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).startsWith("NotObject"))
>> .reduce((first, second) -> second)
>> .orElse(-1);
>> System.out.println(index2); // Output: 2
>>
>>
>> I believe these additions would greatly enhance the functionality and
>> flexibility of the `List` interface, making it more powerful and
>> user-friendly. I look forward to your feedback and am open to making any
>> necessary changes bas...
>
> Evemose has updated the pull request incrementally with one additional commit
> since the last revision:
>
> reverted code style changes in Vector
> > > I noticed that most (if not all) methods don't ensure non-nullability of
> > > `filter` so NPE would only be thrown if the list is not empty.
> >
> >
> > Yeah, thats true. not sure if it has to throw NPE even if list is emply
>
> Yes, it does. If it shouldn't, then why isn't code in the java.util.List is
> like this:
>
> ```java
> default int findIndex(Predicate filter) {
> ListIterator iterator = listIterator();
> if (!iterator.hasNext()) return -1;
> Objects.requireNonNull(filter);
> int index = 0;
> do {
> if (filter.test(iterator.next()))
> return index;
> index++;
> } while (iterator.hasNext());
> return -1;
> }
> ```
>
> Also see methods like `removveIf`. All methods should do checks even if they
> essentially do nothing.
ok, i will fix this in a while
-
PR Comment: https://git.openjdk.org/jdk/pull/18639#issuecomment-2067357432
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v11]
On Fri, 19 Apr 2024 22:20:48 GMT, Evemose wrote:
> > I noticed that most (if not all) methods don't ensure non-nullability of
> > `filter` so NPE would only be thrown if the list is not empty.
>
> Yeah, thats true. not sure if it has to throw NPE even if list is emply
Yes, it does. If it shouldn't, then why isn't code in the java.util.List is
like this:
default int findIndex(Predicate filter) {
ListIterator iterator = listIterator();
if (!iterator.hasNext()) return -1;
Objects.requireNonNull(filter);
int index = 0;
do {
if (filter.test(iterator.next()))
return index;
index++;
} while (iterator.hasNext());
return -1;
}
Also see methods like `removveIf`. All methods should do checks even if they
essentially do nothing.
-
PR Comment: https://git.openjdk.org/jdk/pull/18639#issuecomment-2067354358
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v11]
On Fri, 19 Apr 2024 21:36:42 GMT, xxDark wrote: > I noticed that most (if not all) methods don't ensure non-nullability of > `filter` so NPE would only be thrown if the list is not empty. Yeah, thats true. not sure if it has to throw NPE even if list is emply - PR Comment: https://git.openjdk.org/jdk/pull/18639#issuecomment-2067350248
Re: RFR: 8329760: Add indexOf(Predicate filter) to java.util.List interface [v12]
> **Subject**
> Addition of Predicate-based `indexOf` and `lastIndexOf` methods to
> `java.util.List`
>
> **Motivation**
> The motivation behind this proposal is to enhance the functionality of the
> `List` interface by providing a more flexible way to find the index of an
> element. Currently, the `indexOf` and `lastIndexOf` methods only accept an
> object as a parameter. This limits the flexibility of these methods as they
> can only find the index of exact object matches.
>
> The proposed methods would accept a `Predicate` as a parameter, allowing
> users to define a condition that the desired element must meet. This would
> provide a more flexible and powerful way to find the index of an element in a
> list.
>
> Here is a brief overview of the changes made in this pull request:
>
> 1. Added the `indexOf(Predicate filter)` method to the `List`
> interface.
> 2. Added the `lastIndexOf(Predicate filter)` method to the `List`
> interface.
> 3. Implemented these methods in all non-abstract classes that implement the
> `List` interface.
>
> The changes have been thoroughly tested to ensure they work as expected and
> do not introduce any regressions. The test cases cover a variety of scenarios
> to ensure the robustness of the implementation.
>
> For example, consider the following test case:
>
> List list = new ArrayList<>();
> list.add("Object one");
> list.add("NotObject two");
> list.add("NotObject three");
>
> int index1 = list.indexOf(s -> s.contains("ct t"));
> System.out.println(index1); // Expected output: 1
> int index2 = list.lastIndexOf(s -> s.startsWith("NotObject"));
> System.out.println(index2); // Expected output: 2
>
>
> Currently, to achieve the same result, we would have to use a more verbose
> approach:
>
> int index1 = IntStream.range(0, list.size())
> .filter(i -> list.get(i).contains("ct t"))
> .findFirst()
> .orElse(-1);
> System.out.println(index1); // Output: 1
> int index2 = IntStream.range(0, list.size())
> .filter(i -> list.get(i).startsWith("NotObject"))
> .reduce((first, second) -> second)
> .orElse(-1);
> System.out.println(index2); // Output: 2
>
>
> I believe these additions would greatly enhance the functionality and
> flexibility of the `List` interface, making it more powerful and
> user-friendly. I look forward to your feedback and am open to making any
> necessary changes based on your suggestions.
>
> Thank you for considering this proposal.
>
> Best regards
>
> PS: In ...
Evemose has updated the pull request incrementally with three additional
commits since the last revision:
- Added Objects import to sun List
- Replaced on-demand import in com.sunList
- added non-null assertions
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18639/files
- new: https://git.openjdk.org/jdk/pull/18639/files/de6e18e7..349ee6bd
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18639&range=11
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18639&range=10-11
Stats: 29 lines in 8 files changed: 27 ins; 0 del; 2 mod
Patch: https://git.openjdk.org/jdk/pull/18639.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18639/head:pull/18639
PR: https://git.openjdk.org/jdk/pull/18639
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output I agree; the launcher uses two different modes for discovery. If you want to troubleshoot, knowing which of these are attempted is helpful. @AlanBateman If this is not acceptable to add, I must once again iterate my question: What is the use case for the tracing functionality then? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072349646
Re: RFR: 8327858: Improve spliterator and forEach for single-element immutable collections [v2]
On Thu, 21 Mar 2024 18:01:38 GMT, Chen Liang wrote: >> Please review this patch that: >> 1. Implemented `forEach` to optimize for 1 or 2 element collections. >> 2. Implemented `spliterator` to optimize for a single element. >> >> The default implementations for multiple-element immutable collections are >> fine as-is, specializing implementation doesn't provide much benefit. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 13 commits: > > - Use the improved form in forEach > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Null checks should probably be in the beginning... > - mark implicit null checks > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Copyright year, revert changes for non-few element collections > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Merge branch 'feature/imm-coll-stream' of > https://github.com/liachmodded/jdk into feature/imm-coll-stream > - Spliterator for 12, iterate/forEach benchmark > - fix comments > - ... and 3 more: https://git.openjdk.org/jdk/compare/d5b95a0e...69bd0e9c Do we need additional tests or are these modifications already covered by the existing tests? - PR Comment: https://git.openjdk.org/jdk/pull/15834#issuecomment-2072289455
Re: Addition of Predicate-based findIndex and findLastIndex methods to java.util.List
Hello there! As I see, your approach finds all matching indexes, not
just first. This would be much more suitable using enumerated streams.
Discussion about them has been moved to another thread (see
EnumeratedStream subject in mails).
Few messages above I provided benchmarks indicating that for one element,
list-based approach drastically outperforms any stream-based, so if you
would like to contribute something to enumerated streams discussion
specifically, you are welcome, but I don't think such dramatic
performance loss is worth the flexibility your approach provides.
However, a gatherer-based enumerated streams approach has been discussed,
and your implementation looks really concie. I like it.
PS: little offtop regarding enumerated streams. In compiler-dev, I have
proposed to introduce extension methods (workin implementation is already
present). Those extensions could really ease Stream API development as well
as many others and also provide a more comfortable interface for Java
users. However, the compiler maintenance team seems to not even consider my
proposal (or at least I didn't get any feedback besides that it was
rejected previously, even though I provided some argumentation about why
pros outweigh cons of such proposal). If any core-libs-dev mainaiter or
enthusiast feels like such feature could be helpful for them, I encourage
you to state that in a related topic so that the compiler team knows there
is demand for extension methods.Thanks for your time!
вт, 23 апр. 2024 г. в 15:57, Viktor Klang :
> Sadly, there is no way to define a short-circuiting collector :(
>
>
> That was my first course of exploration for JEP-461, and unfortunately I
> found no great way of retrofitting that behavior, especially not in a
> backwards-compatibility-safe way.
>
> You can have a short-circuiting Gatherer like this
>Gatherer findIndex(Predicate predicate) {
> return Gatherer.ofSequential(
> () -> new Object() { int index; },
> Integrtor.ofGreedy((state, element, downstream) -> {
> var index = state.index++;
> if (predicate.test(element)) {
> return downstream.push(index);
> }
> return true;
> }));
> }
>
>
> Since this would ne a short-circuiting Gatherer, it would not qualify as
> Greedy.
>
> With that said, you could create something like:
>
> public static Gatherer indiciesWhere(Predicate p) {
>class Index { long at; }
>return Gatherer.ofSequential(
>Index::new,
>Integrator.ofGreedy((idx, e, d) -> {
> var current = idx.at++;
> return !p.test(e) || d.push(current);
>})
>);
> }
>
>
> So you could do: list.stream().gather(indiciesWhere(predicate)).findFirst()
>
> Cheers,
> √
>
>
> *Viktor Klang*
> Software Architect, Java Platform Group
> Oracle
>
> --
> *From:* core-libs-dev on behalf of Remi
> Forax
> *Sent:* Friday, 19 April 2024 19:47
> *To:* ІП-24 Олександр Ротань
> *Cc:* core-libs-dev
> *Subject:* Re: Addition of Predicate-based findIndex and findLastIndex
> methods to java.util.List
>
> Hello,
> for me, it seems what you want is Collector on Stream which is able to
> short-circuit,
> so you can write
> list.stream().collect(Collectors.findFirst(s -> s.contains("o")))
> and in reverse
> list.reversed().stream().collect(Collectors.findFirst(s ->
> s.contains("o")))
>
> Using a Stream here is more general and will work with other collections
> like a LinkedHashSet for example.
> Sadly, there is no way to define a short-circuiting collector :(
>
> You can have a short-circuiting Gatherer like this
>Gatherer findIndex(Predicate predicate) {
> return Gatherer.ofSequential(
> () -> new Object() { int index; },
> Integrtor.ofGreedy((state, element, downstream) -> {
> var index = state.index++;
> if (predicate.test(element)) {
> return downstream.push(index);
> }
> return true;
> }));
> }
>
> and use it like this:
> list.stream().gather(findIndex(s ->
> s.contains("o"))).findFirst().orElse(-1);
>
> But it's more verbose.
>
> I wonder if at the same time that the Gatherer API is introduced, the
> Collector API should be enhanced to support short-circuiting collectors ?
>
> regards,
> Rémi
>
>
> --
>
> *From: *"ІП-24 Олександр Ротань"
> *To: *"core-libs-dev"
> *Sent: *Friday, April 19, 2024 5:59:39 PM
> *Subject: *Addition of Predicate-based findIndex and findLastIndex
> methods to java.util.List
>
> Subject
> Addition of Predicate-based findIndex and findLastIndex methods to
> java.util.List
>
> Motivation
> The motivation behind this proposal is to enhance the functionality of the
> List interface by providing a more flexible way to find the index of an
> element. Currently, the indexOf and lastIndexOf methods only accept an
> object as a parameter. This limits the flexibility of these methods
RFR: 8325373: Improve StackCounter error reporting for bad switch cases
ClassFile API `StackMapGenerator` attaches print or hex dump of the method to an error message. However there is no such attachment when the stack maps generation is turned off. This patch moves class print/dump to `impl.Util::dumpMethod`, so it is shared by `StackMapGenerator` and `StackCounter` to provide the same level of details in case of an error. Please review. Thank you, Adam - Commit messages: - 8325373: Improve StackCounter error reporting for bad switch cases Changes: https://git.openjdk.org/jdk/pull/18914/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18914&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325373 Stats: 97 lines in 3 files changed: 56 ins; 31 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/18914.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18914/head:pull/18914 PR: https://git.openjdk.org/jdk/pull/18914
Re: RFR: 8327858: Improve spliterator and forEach for single-element immutable collections [v2]
On Thu, 21 Mar 2024 18:01:38 GMT, Chen Liang wrote: >> Please review this patch that: >> 1. Implemented `forEach` to optimize for 1 or 2 element collections. >> 2. Implemented `spliterator` to optimize for a single element. >> >> The default implementations for multiple-element immutable collections are >> fine as-is, specializing implementation doesn't provide much benefit. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 13 commits: > > - Use the improved form in forEach > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Null checks should probably be in the beginning... > - mark implicit null checks > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Copyright year, revert changes for non-few element collections > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Merge branch 'feature/imm-coll-stream' of > https://github.com/liachmodded/jdk into feature/imm-coll-stream > - Spliterator for 12, iterate/forEach benchmark > - fix comments > - ... and 3 more: https://git.openjdk.org/jdk/compare/d5b95a0e...69bd0e9c @stuart-marks & @AlanBateman Specialization of forEach() and spliterator()? - PR Comment: https://git.openjdk.org/jdk/pull/15834#issuecomment-2072240066
Re: Addition of Predicate-based findIndex and findLastIndex methods to java.util.List
Sadly, there is no way to define a short-circuiting collector :(
That was my first course of exploration for JEP-461, and unfortunately I found
no great way of retrofitting that behavior, especially not in a
backwards-compatibility-safe way.
You can have a short-circuiting Gatherer like this
Gatherer findIndex(Predicate predicate) {
return Gatherer.ofSequential(
() -> new Object() { int index; },
Integrtor.ofGreedy((state, element, downstream) -> {
var index = state.index++;
if (predicate.test(element)) {
return downstream.push(index);
}
return true;
}));
}
Since this would ne a short-circuiting Gatherer, it would not qualify as Greedy.
With that said, you could create something like:
public static Gatherer indiciesWhere(Predicate p) {
class Index { long at; }
return Gatherer.ofSequential(
Index::new,
Integrator.ofGreedy((idx, e, d) -> {
var current = idx.at++;
return !p.test(e) || d.push(current);
})
);
}
So you could do: list.stream().gather(indiciesWhere(predicate)).findFirst()
Cheers,
√
Viktor Klang
Software Architect, Java Platform Group
Oracle
From: core-libs-dev on behalf of Remi Forax
Sent: Friday, 19 April 2024 19:47
To: ІП-24 Олександр Ротань
Cc: core-libs-dev
Subject: Re: Addition of Predicate-based findIndex and findLastIndex methods to
java.util.List
Hello,
for me, it seems what you want is Collector on Stream which is able to
short-circuit,
so you can write
list.stream().collect(Collectors.findFirst(s -> s.contains("o")))
and in reverse
list.reversed().stream().collect(Collectors.findFirst(s -> s.contains("o")))
Using a Stream here is more general and will work with other collections like a
LinkedHashSet for example.
Sadly, there is no way to define a short-circuiting collector :(
You can have a short-circuiting Gatherer like this
Gatherer findIndex(Predicate predicate) {
return Gatherer.ofSequential(
() -> new Object() { int index; },
Integrtor.ofGreedy((state, element, downstream) -> {
var index = state.index++;
if (predicate.test(element)) {
return downstream.push(index);
}
return true;
}));
}
and use it like this:
list.stream().gather(findIndex(s -> s.contains("o"))).findFirst().orElse(-1);
But it's more verbose.
I wonder if at the same time that the Gatherer API is introduced, the Collector
API should be enhanced to support short-circuiting collectors ?
regards,
Rémi
From: "ІП-24 Олександр Ротань"
To: "core-libs-dev"
Sent: Friday, April 19, 2024 5:59:39 PM
Subject: Addition of Predicate-based findIndex and findLastIndex methods to
java.util.List
Subject
Addition of Predicate-based findIndex and findLastIndex methods to
java.util.List
Motivation
The motivation behind this proposal is to enhance the functionality of the List
interface by providing a more flexible way to find the index of an element.
Currently, the indexOf and lastIndexOf methods only accept an object as a
parameter. This limits the flexibility of these methods as they can only find
the index of exact object matches.
Here I want to propose methods that would accept a Predicate as a parameter,
allowing users to define a condition that the desired element must meet. This
would provide a more flexible and powerful way to find the index of an element
in a list.
The changes I am proposing are implemented in this PR:
https://github.com/openjdk/jdk/pull/18639. Here is a brief overview of the
changes made in this pull request:
Added the findIndex (Predicate filter) method to the List interface.
Added the findLastIndex (Predicate filter) method to the List
interface.
Implemented these methods in all non-abstract classes that implement the List
interface, as well as List itself (default impl).
The changes have been thoroughly tested to ensure they work as expected and do
not introduce any regressions. The test cases cover a variety of scenarios to
ensure the robustness of the implementation.
For example, consider the following test case:
List list = new ArrayList<>();
list.add("Object one");
list.add("NotObject two");
list.add("NotObject three");
int index1 = list.findIndex(s -> s.contains("ct t"));
System.out.println(index1); // Expected output: 1
int index2 = list. findLastIndex(s -> s.startsWith("NotObject"));
System.out.println(index2); // Expected output: 2
Currently, to achieve the same result, we would have to use a more verbose
approach:
int index1 = IntStream.range(0, list.size())
.filter(i -> list.get(i).contains("ct t"))
.findFirst()
.orElse(-1);
System.out.println(index1); // Output: 1
int index2 = IntStream.range(0, list.size())
.filter(i -> list.get(i).startsWith("NotOb
Re: RFR: 7036144: GZIPInputStream readTrailer uses faulty available() test for end-of-stream [v9]
On Sun, 17 Mar 2024 22:53:55 GMT, Archie Cobbs wrote: >> `GZIPInputStream`, when looking for a concatenated stream, relies on what >> the underlying `InputStream` says is how many bytes are `available()`. But >> this is inappropriate because `InputStream.available()` is just an estimate >> and is allowed (for example) to always return zero. >> >> The fix is to ignore what's `available()` and just proceed and see what >> happens. If fewer bytes are available than required, the attempt to extend >> to another stream is canceled just as it was before, e.g., when the next >> stream header couldn't be read. > > Archie Cobbs 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: > > - Merge branch 'master' into JDK-7036144 > - Back-out Javadoc addition; to be added in a separate issue. > - Document the handling of concatenated streams. > - Merge branch 'master' into JDK-7036144 > - Merge branch 'master' into JDK-7036144 > - Merge branch 'master' into JDK-7036144 > - Address third round of review comments. > - Address second round of review comments. > - Address review comments. > - Fix bug in GZIPInputStream when underlying available() returns short. Hello Archie, we forgot to create a release note for this one (there's still time). Would you be willing to create one, following the instructions here https://openjdk.org/guide/#release-notes? If you need any help, let us know. One of us will review that release note before you can Resolve it to Delivered. - PR Comment: https://git.openjdk.org/jdk/pull/17113#issuecomment-2072214303
Usage of iconv()
Du to a glibc security alert about a charset in iconv() I checked OpenJDK (since I was quite sure encoding is handled in JCL), however there are a few utilities (related to libinstrument and splash Screens) which use iconv. If I see it correctly it’s mostly used for utf8 so it should not expose this particular globe weakness, but I still wonder if that dependency is needed. For some platforms like AIX it even drags on an additional library dependency. (Not to mention different charger tables and especially ugly locale dependencies for containers). Gruß Bernd — https://bernd.eckenfels.net
RFR: 8322847: java.lang.classfile.BufWriter should specify @throws for its writeXXX methods
This patch adds missing `@throws` javadoc annotations to `java.lang.classfile.BufWriter`. Please review. Thank you, Adam - Commit messages: - 8322847: java.lang.classfile.BufWriter should specify @throws for its writeXXX methods Changes: https://git.openjdk.org/jdk/pull/18913/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18913&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8322847 Stats: 7 lines in 1 file changed: 5 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18913.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18913/head:pull/18913 PR: https://git.openjdk.org/jdk/pull/18913
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Mon, 22 Apr 2024 17:19:14 GMT, Magnus Ihse Bursie wrote: > But that sounds like a very special case. Not sure if it is really such a special case. Currently both modes (getting the image path from launcher binary path , and getting the image path from 'the dll' / GetApplicationHomeFromDll (libjli.so on Linux) exist and are to my knowledge supported. At least one jtreg test fails when the GetApplicationHomeFromDll does not work any more. So I think it is natural that both modes are also supported/augmented by some tracing. We could probably reduce the trace message(s) added to just one if this is desired . - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072090837
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Mon, 22 Apr 2024 19:49:44 GMT, Brian Burkhalter wrote:
>> Prevent blocking due to a carrier thread not being released when
>> `ByteArrayOutputStream.writeTo` is invoked from a virtual thread.
>
> Brian Burkhalter has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Correct ID in test @bug tag
src/java.base/share/classes/java/io/ByteArrayOutputStream.java line 164:
> 162: public void writeTo(OutputStream out) throws IOException {
> 163: if (Thread.currentThread().isVirtual()) {
> 164: out.write(toByteArray());
Would it be better to avoid calling a public method `toByteArray` encase
subclass is overriding it?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1576078358
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 07:27:02 GMT, Alan Bateman wrote: > > Has the alternative of moving to a j.u.c.Lock (Needs Reentrant or not?) > > been explored/benchmarked? > > Yes, decided not to do because it's just guarding access to a byte[] and any > changes can influence the inlining. Plus, it would need to continue to use > monitors when extended as the subclass may assume synchronization in the > super class. Limiting it to just the problematic writeTo method seem the > least risky. That's true, good points across the board. - PR Comment: https://git.openjdk.org/jdk/pull/18901#issuecomment-2071869067
Re: RFR: 8330686: Fix typos in the DatabaseMetaData javadoc
On Sat, 20 Apr 2024 04:35:05 GMT, Jaikiran Pai wrote: >> @openjdk with issue from where? > > Hello @onacit, it appears a JBS issue has been filed for this > https://bugs.openjdk.org/browse/JDK-8330686. Please change the title of this > PR to `8330686: Fix typos in the DatabaseMetaData javadoc` so that it > triggers the official review process. @jaikiran Done. Thank you. - PR Comment: https://git.openjdk.org/jdk/pull/18860#issuecomment-2071843513
Re: RFR: 8330686: Fix typos in the DatabaseMetaData javadoc [v2]
> Fix typos within the `DatabaseMetaData.java`. Jin Kwon has updated the pull request incrementally with one additional commit since the last revision: [JDK-8330686] Update copyright year - Changes: - all: https://git.openjdk.org/jdk/pull/18860/files - new: https://git.openjdk.org/jdk/pull/18860/files/fbd41746..75f0b111 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18860&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18860&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18860.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18860/head:pull/18860 PR: https://git.openjdk.org/jdk/pull/18860
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O [v2]
On Tue, 23 Apr 2024 08:04:54 GMT, Viktor Klang wrote:
>> 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 five additional
>> commits since the last revision:
>>
>> - Merge
>> - Sync up from loom repo, update copyright headers
>> - Merge
>> - Merge
>> - Initial commit
>
> src/java.base/share/classes/java/io/FileInputStream.java line 211:
>
>> 209: * @param name the name of the file
>> 210: */
>> 211: private void open(String name) throws FileNotFoundException {
>
> If method such as this is private, and only delegates to the 0-suffixed
> native method, would't it be better to just call the 0-suffixed method
> directly?
Historically these native methods were wrapped in order to support
instrumentation, I didn't want to touch in this PR.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18598#discussion_r1575831563
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O [v2]
On Mon, 22 Apr 2024 12:45:53 GMT, Alan Bateman wrote:
>> This change drops the adjustments to the virtual thread scheduler's target
>> parallelism when virtual threads do file operations on files that are opened
>> for buffered I/O. These operations are usually just too short to have any
>> benefit and may have a negative benefit when reading/writing a small number
>> of bytes. There is no change for read/write operations on files opened for
>> direct I/O or when writing to files that are opened with options for
>> synchronized I/O file integrity (O_SYNC/O_DSYNC and equivalents). Sergery
>> Kuksenko is polishing benchmarks that includes this area, this is for a
>> future PR.
>>
>> In addition, the blocker mechanism is updated to handle reentrancy (as can
>> happen if debugging code is added to ForkJoinPool) and preemption when
>> compensating (as can happen when substituting a heap buffer with a direct
>> buffer in some I/O operations). This part is a pre-requisite to the changes
>> to better support object monitor there are more places where preemption is
>> possible and this quickly leads to unbalanced begin/end.
>>
>> The changes have been baking in loom repo for some time.
>
> 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 five additional
> commits since the last revision:
>
> - Merge
> - Sync up from loom repo, update copyright headers
> - Merge
> - Merge
> - Initial commit
src/java.base/share/classes/java/io/FileInputStream.java line 211:
> 209: * @param name the name of the file
> 210: */
> 211: private void open(String name) throws FileNotFoundException {
If method such as this is private, and only delegates to the 0-suffixed native
method, would't it be better to just call the 0-suffixed method directly?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18598#discussion_r1575825688
Integrated: 8330802: Desugar switch in Locale::createLocale
On Mon, 22 Apr 2024 10:34:29 GMT, Claes Redestad wrote: > This switch expression in `Locale::createLocale` is causing a somewhat large > startup regression on my local system. Desugaring to if statements seem like > the right thing to do while we investigate ways to further reduce overheads > in `SwitchBootstraps`. > > These numbers are against a baseline which include #18865 and #18845, which > already improved the situation: > > > Name Cnt Base Error Test Error > Unit Change > Perfstartup-Noop 2040,500 ± 1,942 31,000 ± 2,673 >ms/op 1,31x (p = 0,000*) > :.cycles 143254849,000 ± 3398321,355 102205427,650 ± 2192784,853 > cycles 0,71x (p = 0,000*) > :.instructions 307138448,850 ± 2095834,550 219415574,800 ± 376992,067 > instructions 0,71x (p = 0,000*) > :.taskclock 39,500 ± 1,942 22,500 ± 3,858 > ms 0,57x (p = 0,000*) > * = significant > > > Comparing to a baseline without those recent improvements the overhead was > almost the double: > > Name Cnt Base Error Test Error > Unit Change > Perfstartup-Noop 2050,000 ± 0,000 31,000 ± 2,673 >ms/op 1,61x (p = 0,000*) > :.cycles 187047932,000 ± 3330400,381 102205427,650 ± 2192784,853 > cycles 0,55x (p = 0,000*) > :.instructions 408219060,350 ± 4031173,140 219415574,800 ± 376992,067 > instructions 0,54x (p = 0,000*) > :.taskclock 53,500 ± 4,249 22,500 ± 3,858 > ms 0,42x (p = 0,000*) > * = significant This pull request has now been integrated. Changeset: daa5a4bd Author:Claes Redestad URL: https://git.openjdk.org/jdk/commit/daa5a4bd124d539daa3c67a3e29dcd0eee20c44d Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod 8330802: Desugar switch in Locale::createLocale Reviewed-by: alanb, liach, rriggs, naoto, mchung - PR: https://git.openjdk.org/jdk/pull/18882
Re: RFR: 8330802: Desugar switch in Locale::createLocale [v2]
On Mon, 22 Apr 2024 14:11:41 GMT, Claes Redestad wrote: >> This switch expression in `Locale::createLocale` is causing a somewhat large >> startup regression on my local system. Desugaring to if statements seem like >> the right thing to do while we investigate ways to further reduce overheads >> in `SwitchBootstraps`. >> >> These numbers are against a baseline which include #18865 and #18845, which >> already improved the situation: >> >> >> Name Cnt Base Error Test >> Error Unit Change >> Perfstartup-Noop 2040,500 ± 1,942 31,000 ± >> 2,673ms/op 1,31x (p = 0,000*) >> :.cycles 143254849,000 ± 3398321,355 102205427,650 ± >> 2192784,853 cycles 0,71x (p = 0,000*) >> :.instructions 307138448,850 ± 2095834,550 219415574,800 ± >> 376992,067 instructions 0,71x (p = 0,000*) >> :.taskclock 39,500 ± 1,942 22,500 ± >> 3,858 ms 0,57x (p = 0,000*) >> * = significant >> >> >> Comparing to a baseline without those recent improvements the overhead was >> almost the double: >> >> Name Cnt Base Error Test >> Error Unit Change >> Perfstartup-Noop 2050,000 ± 0,000 31,000 ± >> 2,673ms/op 1,61x (p = 0,000*) >> :.cycles 187047932,000 ± 3330400,381 102205427,650 ± >> 2192784,853 cycles 0,55x (p = 0,000*) >> :.instructions 408219060,350 ± 4031173,140 219415574,800 ± >> 376992,067 instructions 0,54x (p = 0,000*) >> :.taskclock 53,500 ± 4,249 22,500 ± >> 3,858 ms 0,42x (p = 0,000*) >> * = significant > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Remove InternalError in favor of CCE Thanks for reviewing, everyone! - PR Comment: https://git.openjdk.org/jdk/pull/18882#issuecomment-2071666342
RFR: 8330615: avoid signed integer overflows in zip_util.c readCen / hashN
In the hashN usages of readCen from zip_util.c we see a lot of signed integer overflows. For example in the java/util jtreg tests those are easily reproducable when compiling with -ftrapv (clang/gcc toolchains). While those overflows never seem to cause crashes or similar errors, they are unwanted because signed integer overflows in C cause undefined behavior. See https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Signed-Overflow.html > > For signed integers, the result of overflow in C is in principle undefined, > meaning that anything whatsoever could happen. > Therefore, C compilers can do optimizations that treat the overflow case with > total unconcern. So we might still get unwanted results (maybe bad/strange hashing, depending on compiler and optimization level). Compilation with -ftrapv causes/triggers this SIGILL on macOS showing the issue : # Problematic frame: # C [libzip.dylib+0x6362] hashN+0x32 # Stack: [0x7c496000,0x7c596000], sp=0x7c5957e0, free space=1021k Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) C [libzip.dylib+0x6362] hashN+0x32 C [libzip.dylib+0x5d5e] readCEN+0xd2e C [libzip.dylib+0x4ee0] ZIP_Put_In_Cache0+0x160 V [libjvm.dylib+0x544b1e] ClassLoader::open_zip_file(char const*, char**, JavaThread*)+0x3e V [libjvm.dylib+0x543fec] ClassLoader::create_class_path_entry(JavaThread*, char const*, stat const*, bool, bool)+0x6c V [libjvm.dylib+0x543833] ClassLoader::setup_bootstrap_search_path_impl(JavaThread*, char const*)+0xf3 V [libjvm.dylib+0x54819b] classLoader_init1()+0x1b V [libjvm.dylib+0x92602a] init_globals()+0x3a V [libjvm.dylib+0x12b3b74] Threads::create_vm(JavaVMInitArgs*, bool*)+0x314 V [libjvm.dylib+0xa848f4] JNI_CreateJavaVM+0x64 C [libjli.dylib+0x4483] JavaMain+0x123 C [libjli.dylib+0x7529] ThreadJavaMain+0x9 C [libsystem_pthread.dylib+0x68fc] _pthread_start+0xe0 C [libsystem_pthread.dylib+0x2443] thread_start+0xf - Commit messages: - JDK-8330615 Changes: https://git.openjdk.org/jdk/pull/18908/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18908&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8330615 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18908.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18908/head:pull/18908 PR: https://git.openjdk.org/jdk/pull/18908
RFR: 8330684: ClassFile API runs into StackOverflowError while parsing certain class' bytes
ClassFile API dives into the nested constant pool entries without type restrictions, while parsing a class file. Validation of the entry is performed post-parsing. Specifically corrupted constant pool entry may cause infinite loop during parsing and throws SOE. This patch resolves the issue by providing specific implementations for the nested CP entries parsing, instead of sharing the common (post-checking) code. Added test simulates the situation on inner-looped method reference entry. Please review. Thank you, Adam - Commit messages: - added bug# - 8330684: ClassFile API runs into StackOverflowError while parsing certain class' bytesactory.java Changes: https://git.openjdk.org/jdk/pull/18907/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18907&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8330684 Stats: 84 lines in 2 files changed: 60 ins; 5 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/18907.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18907/head:pull/18907 PR: https://git.openjdk.org/jdk/pull/18907
Re: RFR: 8330686: Fix typos in the DatabaseMetaData javadoc
On Fri, 19 Apr 2024 11:33:48 GMT, Jin Kwon wrote: > Fix typos within the `DatabaseMetaData.java`. These typo fixes look fine to me. Please update the copyright year on the file from `1996, 2022,` to `1996, 2024,` - PR Review: https://git.openjdk.org/jdk/pull/18860#pullrequestreview-2016467011
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 07:11:34 GMT, Viktor Klang wrote: > Has the alternative of moving to a j.u.c.Lock (Needs Reentrant or not?) been > explored/benchmarked? Yes, decided not to do because it's just guarding access to a byte[] and any changes can influence the inlining. Plus, it would need to continue to use monitors when extended as the subclass may assume synchronization in the super class. Limiting it to just the problematic writeTo method seem the least risky. - PR Comment: https://git.openjdk.org/jdk/pull/18901#issuecomment-2071613230
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Mon, 22 Apr 2024 19:49:44 GMT, Brian Burkhalter wrote: >> Prevent blocking due to a carrier thread not being released when >> `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > Correct ID in test @bug tag Has the alternative of moving to a j.u.c.Lock (Needs Reentrant or not?) been explored/benchmarked? - PR Comment: https://git.openjdk.org/jdk/pull/18901#issuecomment-2071589121
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O [v2]
On Mon, 22 Apr 2024 12:45:53 GMT, Alan Bateman wrote: >> This change drops the adjustments to the virtual thread scheduler's target >> parallelism when virtual threads do file operations on files that are opened >> for buffered I/O. These operations are usually just too short to have any >> benefit and may have a negative benefit when reading/writing a small number >> of bytes. There is no change for read/write operations on files opened for >> direct I/O or when writing to files that are opened with options for >> synchronized I/O file integrity (O_SYNC/O_DSYNC and equivalents). Sergery >> Kuksenko is polishing benchmarks that includes this area, this is for a >> future PR. >> >> In addition, the blocker mechanism is updated to handle reentrancy (as can >> happen if debugging code is added to ForkJoinPool) and preemption when >> compensating (as can happen when substituting a heap buffer with a direct >> buffer in some I/O operations). This part is a pre-requisite to the changes >> to better support object monitor there are more places where preemption is >> possible and this quickly leads to unbalanced begin/end. >> >> The changes have been baking in loom repo for some time. > > 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 five additional > commits since the last revision: > > - Merge > - Sync up from loom repo, update copyright headers > - Merge > - Merge > - Initial commit Hello Alan, the latest changes look fine to me. They also address the review comments from the previous iteration. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18598#pullrequestreview-2016427528
Re: RFR: 8330182: Start of release updates for JDK 24 [v2]
On Wed, 17 Apr 2024 05:43:12 GMT, Joe Darcy wrote: >> Get JDK 24 underway. > > Joe Darcy has updated the pull request incrementally with two additional > commits since the last revision: > > - Correct release date as observed in review feedback. > - Improve javadoc of class file update. LGTM Thanks test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java line 1: > 1: /* There are further updates to this test in the pipeline (new deprecated flags in 23) so you will need to keep updating to reflect that. - Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18787#pullrequestreview-2016422206 PR Review Comment: https://git.openjdk.org/jdk/pull/18787#discussion_r1575750313
