Re: RFR: 8341260: Add Float16 to jdk.incubator.vector [v11]
On Wed, 30 Oct 2024 05:25:47 GMT, Joe Darcy wrote:
>> Port of Float16 from java.lang in the lworld+fp16 branch to
>> jdk.incubabor.vector.
>
> Joe Darcy has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Respond to review feedback.
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
431:
> 429: * @seejava.lang.Float#valueOf(String)
> 430: */
> 431: public static Float16 valueOf(String s) throws NumberFormatException
> {
The current implementation throws when the input is `"NaN"`, with or without an
optional sign.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1836721773
Re: RFR: 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits
On Mon, 11 Nov 2024 12:34:44 GMT, Shaojin Wen wrote: > 8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, > > I can't reproduce the problem on a MacBook M1 Max, but I agree that more > testing is needed, so let's roll it back first. Thanks for the BACKOUT, looks right. - Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22012#pullrequestreview-2427435024
Re: RFR: 8343933: Add a MemorySegment::fill benchmark with varying sizes [v2]
> This PR proposes to add a new `MemorySegment::fill" benchmark where the byte > size of the segments varies. Per Minborg 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 six additional commits since the last revision: - Add mixed test - Merge branch 'master' into ms-fill-bench-sizes - Use static arrays - Revert changes from other branch - Add benchmark - Add benchmarks - Changes: - all: https://git.openjdk.org/jdk/pull/22010/files - new: https://git.openjdk.org/jdk/pull/22010/files/8760cc87..7734ee3b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=22010&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=22010&range=00-01 Stats: 12724 lines in 321 files changed: 9622 ins; 1868 del; 1234 mod Patch: https://git.openjdk.org/jdk/pull/22010.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22010/head:pull/22010 PR: https://git.openjdk.org/jdk/pull/22010
Re: RFR: 8336707: Contention of ForkJoinPool grows when stealing works [v22]
> This addresses tendencies in previous update to increase fencing, scanning, > and signalling that can increase contention, and slow down performance > especially on ARM platforms. It also uses more ARM-friendly constructions to > reduce overhead (leading to several changes that all of the same form), Doug Lea 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 41 additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8336707 - Merge branch 'openjdk:master' into JDK-8336707 - Minor improvements - Merge branch 'openjdk:master' into JDK-8336707 - Add CLEANED runState - Merge branch 'openjdk:master' into JDK-8336707 - More shutdown streamlining - Don't report termination while cancellations in progress - Reduce read contention - Merge branch 'openjdk:master' into JDK-8336707 - ... and 31 more: https://git.openjdk.org/jdk/compare/2831441d...7d15d64f - Changes: - all: https://git.openjdk.org/jdk/pull/21507/files - new: https://git.openjdk.org/jdk/pull/21507/files/4db28137..7d15d64f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21507&range=21 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21507&range=20-21 Stats: 5444 lines in 83 files changed: 4777 ins; 470 del; 197 mod Patch: https://git.openjdk.org/jdk/pull/21507.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21507/head:pull/21507 PR: https://git.openjdk.org/jdk/pull/21507
Re: RFR: 8336707: Contention of ForkJoinPool grows when stealing works [v23]
> This addresses tendencies in previous update to increase fencing, scanning, > and signalling that can increase contention, and slow down performance > especially on ARM platforms. It also uses more ARM-friendly constructions to > reduce overhead (leading to several changes that all of the same form), Doug Lea has updated the pull request incrementally with two additional commits since the last revision: - Merge remote-tracking branch 'refs/remotes/origin/JDK-8336707' into JDK-8336707 - Reconcile internal docs; renamings - Changes: - all: https://git.openjdk.org/jdk/pull/21507/files - new: https://git.openjdk.org/jdk/pull/21507/files/7d15d64f..fd19351b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21507&range=22 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21507&range=21-22 Stats: 57 lines in 1 file changed: 15 ins; 0 del; 42 mod Patch: https://git.openjdk.org/jdk/pull/21507.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21507/head:pull/21507 PR: https://git.openjdk.org/jdk/pull/21507
Re: RFR: 8343933: Add a MemorySegment::fill benchmark with varying sizes [v2]
On Mon, 11 Nov 2024 14:50:57 GMT, Per Minborg wrote: >> This PR proposes to add a new `MemorySegment::fill" benchmark where the byte >> size of the segments varies. > > Per Minborg 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 six additional > commits since the last revision: > > - Add mixed test > - Merge branch 'master' into ms-fill-bench-sizes > - Use static arrays > - Revert changes from other branch > - Add benchmark > - Add benchmarks IIUC, this benchmark is designed to test what happens in the case where a bulk operation is executed on a segment whose type is not known e.g. through type profiling (e.g. polluted case). Note that in this case (MIXED) the fact that we can't rely on which segment it is (and hence on whether it has a certain scope or not, and what the base object is) would greatly dominate the cost of branch misprediction, which I saw mentioned here. In extreme cases of profile pollution it is very possible for a method to be optimized under the assumption that e.g. the method is always executed on off-heap segments, and then hit an on-heap segment, which will perform horribly. We have test cases such as these in `LoopOverPolluted[Segments/Buffer]`. It makes sense to test something similar for bulk access too. - PR Comment: https://git.openjdk.org/jdk/pull/22010#issuecomment-2468683925
Re: Question about Streams, Gatherers, and fetching too many elements
Thanks for the workaround. It's running beautifully.
Is there a future where this island concept is extended to the rest of
streams? Tbh, I don't fully understand it.
On Mon, Nov 11, 2024, 9:59 AM Viktor Klang wrote:
> Hi David,
>
> This is the effect of how parallel streams are implemented, where
> different stages, which are not representible as a join-less Spliterator
> are executed as a series of "islands" where the next isn't started until
> the former has completed.
>
> If you think about it, parallelization of a Stream works best when the
> entire data set can be split amongst a set of worker threads, and that sort
> of implies that you want eager pre-fetch of data, so if your dataset does
> not fit in memory, that is likely to lead to less desirable outcomes.
>
> What I was able to do for Gatherers is to implement "gather(…) +
> collect(…)"-fusion so any number of consecutive gather(…)-operations
> immediately followed by a collect(…) is run in the same "island".
>
> So with that said, you could try something like the following:
>
> static Collector *forEach*(Consumer *each*) {
> *return* Collector.of(() -> null, (*v*, *e*) -> each.accept(e), (*l*,
> *r*) -> l, (*v*) -> null, Collector.Characteristics.IDENTITY_FINISH);
> }
>
>
> stream
> .parallel()
> .unordered()
> .gather(Gatherers.windowFixed(BATCH_SIZE))
> .collect(forEach(eachList -> println(eachList.getFirst(;
>
>
> Cheers,
> √
>
>
> *Viktor Klang*
> Software Architect, Java Platform Group
> Oracle
> --
> *From:* core-libs-dev on behalf of David
> Alayachew
> *Sent:* Monday, 11 November 2024 14:52
> *To:* core-libs-dev
> *Subject:* Re: Question about Streams, Gatherers, and fetching too many
> elements
>
> And just to avoid the obvious question, I can hold about 30 batches in
> memory before the Out of Memory error occurs. So this is not an issue of my
> batch size being too high.
>
> But just to confirm, I set the batch size to 1, and it still ran into an
> out of memory error. So I feel fairly confident saying that the Gatherer is
> trying to grab all available data before sending any of it downstream.
>
> On Mon, Nov 11, 2024, 8:46 AM David Alayachew
> wrote:
>
> Hello Core Libs Dev Team,
>
> I was trying out Gatherers for a project at work, and ran into a rather
> sad scenario.
>
> I need to process a large file in batches. Each batch is small enough that
> I can hold it in memory, but I cannot hold the entire file (and thus, all
> of the batches) in memory at once.
>
> Looking at the Gatherers API, I saw windowFixed and thought that it would
> be a great match for my use case.
>
> However, when trying it out, I was disappointed to see that it ran out of
> memory very quickly. Here is my attempt at using it.
>
> stream
> .parallel()
> .unordered()
> .gather(Gatherers.windowFixed(BATCH_SIZE))
> .forEach(eachList -> println(eachList.getFirst()))
> ;
>
> As you can see, I am just splitting the file into batches, and printing
> out the first of each batch. This is purely for example's sake, of course.
> I had planned on building even more functionality on top of this, but I
> couldn't even get past this example.
>
> But anyways, not even a single one of them printed out. Which leads me to
> believe that it's pulling all of them in the Gatherer.
>
> I can get it to run successfully if I go sequentially, but not parallel.
> Parallel gives me that out of memory error.
>
> Is there any way for me to be able to have the Gatherer NOT pull in
> everything while still remaining parallel and unordered?
>
> Thank you for your time and help.
> David Alayachew
>
>
Re: RFR: 8340205: Native linker allows MemoryLayout consisting of only PaddingLayout [v13]
On Wed, 6 Nov 2024 15:25:07 GMT, Jorn Vernee wrote: > Maybe we should also check that padding layouts have natural alignment? The > alignment of padding layouts can affect the alignment of the enclosing > container. This makes sense, but I wonder if that would require a change in the javadoc? - PR Comment: https://git.openjdk.org/jdk/pull/21041#issuecomment-2468691923
Re: RFR: 8338411: Implement JEP 486: Permanently Disable the Security Manager [v3]
On Fri, 8 Nov 2024 21:01:57 GMT, Phil Race wrote: >>> I'd not looked at this test before but when I do the thing I noticed is >>> that createPrivateValue is no longer used. But I don't see a problem with >>> keeping the rest of the test. >> >> @prrace Do I understand correctly that _“`createPrivateValue` is no longer >> used”_ means `MultiUIDefaults` is never used with `ProxyLazyValue`? >> >> The [`MultiUIDefaults` >> class](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/MultiUIDefaults.java) >> is used in `UIManager`: >> >> https://github.com/openjdk/jdk/blob/c82ad845e101bf5d97c0744377d68002907d4a0e/src/java.desktop/share/classes/javax/swing/UIManager.java#L198 > >> > I'd not looked at this test before but when I do the thing I noticed is >> > that createPrivateValue is no longer used. But I don't see a problem with >> > keeping the rest of the test. >> >> @prrace Do I understand correctly that _“`createPrivateValue` is no longer >> used”_ means `MultiUIDefaults` is never used with `ProxyLazyValue`? >> >> The [`MultiUIDefaults` >> class](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/MultiUIDefaults.java) >> is used in `UIManager`: >> >> https://github.com/openjdk/jdk/blob/c82ad845e101bf5d97c0744377d68002907d4a0e/src/java.desktop/share/classes/javax/swing/UIManager.java#L198 > > I think I was just saying there appeared to be dead code in the test. > > @prrace Do I understand correctly that _“`createPrivateValue` is no longer > > used”_ means `MultiUIDefaults` is never used with `ProxyLazyValue`? > > I think I was just saying there appeared to be dead code in the test. Hmm… `createPrivateValue` had been called from the `main` method, so it had been used in the test until it was removed in https://github.com/openjdk/jdk/commit/9eb275c4aaf9a88127c5c33e0bf7ca35125f29ea Since `MultiUIDefaults` is still used in `UIManager` and we're keeping the test, I'm for keeping a test for `createPrivateValue` too. - PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1837009964
Integrated: 8342707: Prepare Gatherers for graduation from Preview
On Thu, 24 Oct 2024 15:09:00 GMT, Viktor Klang wrote: > Make final adjustments to drop PreviewFeature and updating the @ since > markers. This pull request has now been integrated. Changeset: ef0dc251 Author:Viktor Klang URL: https://git.openjdk.org/jdk/commit/ef0dc2518e7636cc8a9ca580613ff5edeb4c19fd Stats: 50 lines in 24 files changed: 0 ins; 19 del; 31 mod 8342707: Prepare Gatherers for graduation from Preview Reviewed-by: alanb, liach - PR: https://git.openjdk.org/jdk/pull/21686
Re: RFR: 8343250: ArrayBlockingQueue serialization not thread safe
On Wed, 30 Oct 2024 08:54:55 GMT, kabutz wrote: > The ArrayBlockingQueue has had a readObject() method since Java 7, which > checks invariants of the deserialized object. However, it does not have a > writeObject() method. This means that the ArrayBlockingQueue could be > modified whilst it is being written, resulting in broken invariants. The > readObject() method's invariant checking is not exhaustive, which means that > it is possible to end up with ArrayBlockingQueue instances that contain null > values, leading to a difference between "size()" and how many objects would > be returned with "poll()". > > The ABQ should get a writeObject() method that is locking on the same locks > as the rest of the class. @kabutz In case you missed this: "➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration)." - PR Comment: https://git.openjdk.org/jdk/pull/21783#issuecomment-2469092130
Re: RFR: 8343250: ArrayBlockingQueue serialization not thread safe
On Wed, 30 Oct 2024 08:54:55 GMT, kabutz wrote: > The ArrayBlockingQueue has had a readObject() method since Java 7, which > checks invariants of the deserialized object. However, it does not have a > writeObject() method. This means that the ArrayBlockingQueue could be > modified whilst it is being written, resulting in broken invariants. The > readObject() method's invariant checking is not exhaustive, which means that > it is possible to end up with ArrayBlockingQueue instances that contain null > values, leading to a difference between "size()" and how many objects would > be returned with "poll()". > > The ABQ should get a writeObject() method that is locking on the same locks > as the rest of the class. @kabutz Your change (at version 5dc64f7ee613d2738c4abcf97aa79b81e7727f46) is now ready to be sponsored by a Committer. - PR Comment: https://git.openjdk.org/jdk/pull/21783#issuecomment-2469120159
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v7]
On Mon, 11 Nov 2024 09:51:35 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters 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 eight additional > commits since the last revision: > > - Merge branch 'openjdk:master' into unused > - Neater warning silencer in proc_md.h > - Revert _WIN32 workaround in log_messages.c > - Copyright in VersionInfo.cpp > - Remove neutralLangId in VersionInfo.cpp > - Remove global memHandle, would've liked to keep it as a comment :( > - Merge branch 'master' into unused > - 8342682 Marked as reviewed by cjplummer (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/21616#pullrequestreview-2428256817
Re: [External] : Re: Question about Streams, Gatherers, and fetching too many elements
Good to know, ty vm.
At the very least, I have this workaround. This will meet my needs for now.
I guess my final question would be -- is this type of problem better suited
to something besides parallel streams? Maybe an ExecutorService?
Really, all I am doing is taking a jumbo file, splitting it into batches,
and then doing some work on those batches. My IO speeds are pretty fast,
and the compute work is non-trivial, so there is performance being left on
the table if I give up parallelism. And I am in a position where completion
time is very important to us.
I just naturally assumed parallel streams were the right choice because the
compute work is simple. A pure function that I can break out, and then call
in a map. Once I do that, I just call forEach to write the batches back out
to S3. Maybe I should look into a different part of the std lib instead
because I am using the wrong tool for the job? My nose says
ExecutorService, but I figure I should ask before I dive too deep in.
On Mon, Nov 11, 2024, 2:34 PM Viktor Klang wrote:
> You're most welcome!
>
> In a potential future where all intermediate operations are
> Gatherer-based, and all terminal operations are Collector-based, it would
> just work as expected. But with that said, I'm not sure it is practically
> achievable because some operations might not have the same
> performance-characteristics as before.
>
> Cheers,
> √
>
>
> *Viktor Klang*
> Software Architect, Java Platform Group
> Oracle
> --
> *From:* David Alayachew
> *Sent:* Monday, 11 November 2024 18:32
> *To:* Viktor Klang
> *Cc:* core-libs-dev
> *Subject:* [External] : Re: Question about Streams, Gatherers, and
> fetching too many elements
>
>
> Thanks for the workaround. It's running beautifully.
>
> Is there a future where this island concept is extended to the rest of
> streams? Tbh, I don't fully understand it.
>
> On Mon, Nov 11, 2024, 9:59 AM Viktor Klang
> wrote:
>
> Hi David,
>
> This is the effect of how parallel streams are implemented, where
> different stages, which are not representible as a join-less Spliterator
> are executed as a series of "islands" where the next isn't started until
> the former has completed.
>
> If you think about it, parallelization of a Stream works best when the
> entire data set can be split amongst a set of worker threads, and that sort
> of implies that you want eager pre-fetch of data, so if your dataset does
> not fit in memory, that is likely to lead to less desirable outcomes.
>
> What I was able to do for Gatherers is to implement "gather(…) +
> collect(…)"-fusion so any number of consecutive gather(…)-operations
> immediately followed by a collect(…) is run in the same "island".
>
> So with that said, you could try something like the following:
>
> static Collector *forEach*(Consumer *each*) {
> *return* Collector.of(() -> null, (*v*, *e*) -> each.accept(e), (*l*,
> *r*) -> l, (*v*) -> null, Collector.Characteristics.IDENTITY_FINISH);
> }
>
>
> stream
> .parallel()
> .unordered()
> .gather(Gatherers.windowFixed(BATCH_SIZE))
> .collect(forEach(eachList -> println(eachList.getFirst(;
>
>
> Cheers,
> √
>
>
> *Viktor Klang*
> Software Architect, Java Platform Group
> Oracle
> --
> *From:* core-libs-dev on behalf of David
> Alayachew
> *Sent:* Monday, 11 November 2024 14:52
> *To:* core-libs-dev
> *Subject:* Re: Question about Streams, Gatherers, and fetching too many
> elements
>
> And just to avoid the obvious question, I can hold about 30 batches in
> memory before the Out of Memory error occurs. So this is not an issue of my
> batch size being too high.
>
> But just to confirm, I set the batch size to 1, and it still ran into an
> out of memory error. So I feel fairly confident saying that the Gatherer is
> trying to grab all available data before sending any of it downstream.
>
> On Mon, Nov 11, 2024, 8:46 AM David Alayachew
> wrote:
>
> Hello Core Libs Dev Team,
>
> I was trying out Gatherers for a project at work, and ran into a rather
> sad scenario.
>
> I need to process a large file in batches. Each batch is small enough that
> I can hold it in memory, but I cannot hold the entire file (and thus, all
> of the batches) in memory at once.
>
> Looking at the Gatherers API, I saw windowFixed and thought that it would
> be a great match for my use case.
>
> However, when trying it out, I was disappointed to see that it ran out of
> memory very quickly. Here is my attempt at using it.
>
> stream
> .parallel()
> .unordered()
> .gather(Gatherers.windowFixed(BATCH_SIZE))
> .forEach(eachList -> println(eachList.getFirst()))
> ;
>
> As you can see, I am just splitting the file into batches, and printing
> out the first of each batch. This is purely for example's sake, of course.
> I had planned on building even more functionality on top of this, but I
> couldn't even get past this example.
>
> But anyways, not even a single one of them printed out. Which leads me to
RFR: 8342650: Move getChars to DecimalDigits
This PR is a resubmission after PR #21593 was rolled back, and the unsafe offset overflow issue has been fixed. Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to reduce duplication HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16 Putting these two methods into DecimalDigits can avoid the need to expose them in JavaLangAccess Eliminate duplicate code in BigDecimal This PR will improve the performance of Integer/Long.toString and StringBuilder.append(int/long) scenarios. This is because Unsafe.putByte is used to eliminate array bounds checks, and of course this elimination is safe. In previous versions, in Integer/Long.toString and StringBuilder.append(int/long) scenarios, -COMPACT_STRING performed better than +COMPACT_STRING. This is because StringUTF16.getChars uses StringUTF16.putChar, which is similar to Unsafe.putChar, and there is no bounds check. - Commit messages: - fix unsafe address overflow - add benchmark - remove comments, from @liach - Merge remote-tracking branch 'upstream/master' into int_get_chars_dedup_202410 - fix Helper - fix Helper - fix Helper - unsafe putByte - remove digitPair - fix import - ... and 4 more: https://git.openjdk.org/jdk/compare/5890d943...cd9ba309 Changes: https://git.openjdk.org/jdk/pull/22023/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22023&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8342650 Stats: 757 lines in 12 files changed: 381 ins; 352 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/22023.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22023/head:pull/22023 PR: https://git.openjdk.org/jdk/pull/22023
Re: RFR: 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits
On Mon, 11 Nov 2024 12:34:44 GMT, Shaojin Wen wrote: > 8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, > > I can't reproduce the problem on a MacBook M1 Max, but I agree that more > testing is needed, so let's roll it back first. I have verified that this backout matches a `git revert` of the commit that introduced the change in https://bugs.openjdk.org/browse/JDK-8342650. So on that front, this backout looks OK to me. Alan has noted that Chen is running some tests with this backout. So please wait for that review, before integrating. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22012#pullrequestreview-2427380530
Re: RFR: 8340133: Add concise usage message to the java executable [v7]
On Mon, 11 Nov 2024 08:13:18 GMT, Jan Lahoda wrote:
>> Currently, running `java` without any parameters will lead to an output that
>> is a full `--help`, which is over 100 lines (on my computer at least), and
>> it feels overwhelming. And many people might actually want to run
>> JShell/REPL, not the `java` executable, but it is difficult find out about
>> JShell.
>>
>> The proposal herein is to print a much shorter help, together with a pointer
>> to JShell, when the executable does not know what to do. I.e. there is
>> nothing specified to start, and no option like `--help` is specified. In
>> particular, on my machine, it prints:
>>
>> openjdk 24-internal 2025-03-18
>>
>> Usage: java [java options...] [application arguments...]
>>
>> Where is one of:
>> to execute the main method of a compiled class
>> -jar .jar to execute the main class of a JAR archive
>> -m [/] to execute the main class of a module
>> .java to compile and execute a source-file program
>>
>> Where key java options include:
>> --class-path
>> where is a list of directories and JAR archives to search
>> for class files, separated by ":"
>> --module-path
>> where is a list of directories and JAR archives to search
>> for modules, separated by ":"
>>
>> For additional help on usage: java --help
>> For an interactive Java environment:jshell
>>
>>
>> Hopefully, this may be easier both for people trying to run something, and
>> for people that are really looking for JShell.
>>
>> What do you think?
>>
>> Thanks!
>
> Jan Lahoda has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains 11 additional commits since
> the last revision:
>
> - Merge branch 'master' into JDK-8340133-2
> - Using correct pplaceholders.
> - Adjusting text as suggested.
> - Cleaning up the concise message:
>- using 2 spaces instead of 4,
>- rewording the "for more use --help" part of the message as suggested to
> avoid the word "launcher".
> - Using lowercase for the keys in the help, using 'source-file' program
> instead of 'single-file' program.
> - Using an enum instead of booleans, as suggested.
> - Adjusting the concise help as suggested: 'using main class of a JAR
> archive' and '.jar'/'.java'
> - Adjusting the concise help based on review suggestions.
> - Cleanup.
> - Adjusting/improving the concise help.
> - ... and 1 more: https://git.openjdk.org/jdk/compare/414bcbf6...b4d7b493
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 598:
> 596: static void printConciseUsageMessage(boolean printToStderr) {
> 597: initOutput(printToStderr);
> 598:
> ostream.println(SharedSecrets.getJavaLangAccess().shortVersionString());
What is the reason for printing the short version string at the start of the
short usage message?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21411#discussion_r1836738224
Re: RFR: 8340133: Add concise usage message to the java executable [v7]
On Mon, 11 Nov 2024 14:16:25 GMT, Alan Bateman wrote:
>> Jan Lahoda has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 11 additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into JDK-8340133-2
>> - Using correct pplaceholders.
>> - Adjusting text as suggested.
>> - Cleaning up the concise message:
>>- using 2 spaces instead of 4,
>>- rewording the "for more use --help" part of the message as suggested to
>> avoid the word "launcher".
>> - Using lowercase for the keys in the help, using 'source-file' program
>> instead of 'single-file' program.
>> - Using an enum instead of booleans, as suggested.
>> - Adjusting the concise help as suggested: 'using main class of a JAR
>> archive' and '.jar'/'.java'
>> - Adjusting the concise help based on review suggestions.
>> - Cleanup.
>> - Adjusting/improving the concise help.
>> - ... and 1 more: https://git.openjdk.org/jdk/compare/7f4880aa...b4d7b493
>
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 598:
>
>> 596: static void printConciseUsageMessage(boolean printToStderr) {
>> 597: initOutput(printToStderr);
>> 598:
>> ostream.println(SharedSecrets.getJavaLangAccess().shortVersionString());
>
> What is the reason for printing the short version string at the start of the
> short usage message?
In passing, it may be better to pick "short" or "concise", right now it's a mix
in the method and resource keys.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21411#discussion_r1836751174
Re: RFR: 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits
On Mon, 11 Nov 2024 12:34:44 GMT, Shaojin Wen wrote: > 8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, > > I can't reproduce the problem on a MacBook M1 Max, but I agree that more > testing is needed, so let's roll it back first. CI results look good. - Marked as reviewed by liach (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22012#pullrequestreview-2427501471
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path
On Mon, 11 Nov 2024 10:20:02 GMT, Severin Gehwolf wrote: > > In the above script, a containerized process (/bin/sh) is moved to cgroup > > /test before /jdk/bin/java gets executed. Java inherits cgroup /test from > > its parent process, its _root will be /docker/, cgroup_path > > will be /test. > > OK, but why is https://bugs.openjdk.org/browse/JDK-8322420 not in effect in > such a case? Answering my own question. Because the `set_subsystem_path()` function for cg v1 in this unusual setup returns `null`. [0.001s][trace][os,container] OSContainer::init: Initializing Container Support [0.001s][debug][os,container] Detected optional pids controller entry in /proc/cgroups [0.002s][debug][os,container] Detected cgroups hybrid or legacy hierarchy, using cgroups v1 controllers [0.002s][trace][os,container] Adjusting controller path for memory: (null) [0.002s][debug][os,container] read_string: subsystem path is null [0.002s][trace][os,container] Memory Limit failed: -2 [0.002s][debug][os,container] read_string: subsystem path is null [0.002s][trace][os,container] Memory Limit failed: -2 [0.002s][trace][os,container] No lower limit found for memory in hierarchy /sys/fs/cgroup/memory, adjusting to original path /test [0.002s][debug][os,container] OSContainer::init: is_containerized() = true because all controllers are mounted read-only (container case) [0.003s][trace][os,container] Path to /cpu.cfs_quota_us is /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us [0.003s][trace][os,container] CPU Quota is: -1 [0.003s][trace][os,container] Path to /cpu.cfs_period_us is /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us [0.003s][trace][os,container] CPU Period is: 10 [0.003s][trace][os,container] OSContainer::active_processor_count: 12 [0.003s][trace][os,container] CgroupSubsystem::active_processor_count (cached): 12 [0.003s][trace][os,container] total physical memory: 67163226112 [0.003s][debug][os,container] read_string: subsystem path is null [0.003s][trace][os,container] Memory Limit failed: -2 [0.005s][trace][os,container] CgroupSubsystem::active_processor_count (cached): 12 [0.021s][trace][os,container] CgroupSubsystem::active_processor_count (cached): 12 openjdk 24-internal 2025-03-18 OpenJDK Runtime Environment (build 24-internal-adhoc.sgehwolf.jdk-jdk) OpenJDK 64-Bit Server VM (build 24-internal-adhoc.sgehwolf.jdk-jdk, mixed mode, sharing) On cg v2, on the other hand, `set_subsystem_path()` will never set the path to a `null` value. - PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2468417142
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v3]
On Thu, 7 Nov 2024 22:31:21 GMT, Sergey Chernyshev wrote: >> Cgroup V1 subsustem fails to initialize mounted controllers properly in >> certain cases, that may lead to controllers left undetected/inactive. We >> observed the behavior in CloudFoundry deployments, it affects also host >> systems. >> >> The relevant /proc/self/mountinfo line is >> >> >> 2207 2196 0:43 >> /system.slice/garden.service/garden/good/2f57368b-0eda-4e52-64d8-af5c >> /sys/fs/cgroup/cpu,cpuacct ro,nosuid,nodev,noexec,relatime master:25 - >> cgroup cgroup rw,cpu,cpuacct >> >> >> /proc/self/cgroup: >> >> >> 11:cpu,cpuacct:/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c >> >> >> Here, Java runs inside containerized process that is being moved cgroups due >> to load balancing. >> >> Let's examine the condition at line 64 here >> https://github.com/openjdk/jdk/blob/55a7cf14453b6cd1de91362927b2fa63cba400a1/src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp#L59-L72 >> It is always FALSE and the branch is never taken. The issue was spotted >> earlier by @jerboaa in >> [JDK-8288019](https://bugs.openjdk.org/browse/JDK-8288019). >> >> The original logic was intended to find the common prefix of `_root`and >> `cgroup_path` and concatenate the remaining suffix to the `_mount_point` >> (lines 67-68). That could lead to the following results: >> >> Example input >> >> _root = "/a" >> cgroup_path = "/a/b" >> _mount_point = "/sys/fs/cgroup/cpu,cpuacct" >> >> >> result _path >> >> "/sys/fs/cgroup/cpu,cpuacct/b" >> >> >> Here, cgroup_path comes from /proc/self/cgroup 3rd column. The man page >> (https://man7.org/linux/man-pages/man7/cgroups.7.html#NOTES) for control >> groups states: >> >> >> ... >>/proc/pid/cgroup (since Linux 2.6.24) >> This file describes control groups to which the process >> with the corresponding PID belongs. The displayed >> information differs for cgroups version 1 and version 2 >> hierarchies. >> For each cgroup hierarchy of which the process is a >> member, there is one entry containing three colon- >> separated fields: >> >> hierarchy-ID:controller-list:cgroup-path >> >> For example: >> >> 5:cpuacct,cpu,cpuset:/daemons >> ... >> [3] This field contains the pathname of the control group >>in the hierarchy to which the process belongs. This >>pathname is relative to the mount point of the >>hierarchy. >> >> >> This explicitly states the "pathname is relative t... > > Sergey Chernyshev 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 four additional > commits since the last revision: > > - Merge branch 'master' into JDK-8343191 > - patch reimplemented > - fix the logic that skips duplicate controller's mount points > - 8343191: Cgroup v1 subsystem fails to set subsystem path So on cg v1 you start out and end with a `subsystem_path() == null` and on cg v2 you start out and end with a `subsystem_path() == /../../../../../../test`. In both cases the memory limit of 400m won't be detected. - PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2468488527
Re: RFR: 8333796: Add missing serialization functionality to sun.reflect.ReflectionFactory [v8]
On Tue, 22 Oct 2024 12:49:08 GMT, David M. Lloyd wrote: >> Issue [JDK-8164908](https://bugs.openjdk.org/browse/JDK-8164908) added >> support for functionality required to continue to support IIOP and custom >> serializers in light of additional module-based restrictions on reflection. >> It was expected that these libraries would use `sun.misc.Unsafe` in order to >> access fields of serializable classes. However, with JEP 471, the methods >> necessary to do this are being removed. >> >> To allow these libraries to continue to function, it is proposed to add two >> methods to `sun.reflect.ReflectionFactory` which will allow serialization >> libraries to acquire a method handle to generated `readObject`/`writeObject` >> methods which set or get the fields of the serializable class using the >> serialization `GetField`/`PutField` mechanism. These generated methods >> should be used by serialization libraries to serialize and deserialize >> classes which do not have a `readObject`/`writeObject` method or which use >> `ObjectInputStream.defaultReadObject`/`ObjectOutputStream.defaultWriteObject` >> to supplement default serialization. >> >> It is also proposed to add methods which allow for the reading of >> serialization-specific private static final fields from classes which have >> them. >> >> With the addition of these methods, serialization libraries no longer need >> to rely on `Unsafe` for serialization/deserialization activities. >> cc: @AlanBateman > > David M. Lloyd has updated the pull request incrementally with one additional > commit since the last revision: > > Round out the documentation of the new methods to explain the supported and > unsupported cases I think I'm just needing two reviewers now (and a sponsor after that). It's close! - PR Comment: https://git.openjdk.org/jdk/pull/19702#issuecomment-2468558562
Re: RFR: 8336707: Contention of ForkJoinPool grows when stealing works [v24]
> This addresses tendencies in previous update to increase fencing, scanning, > and signalling that can increase contention, and slow down performance > especially on ARM platforms. It also uses more ARM-friendly constructions to > reduce overhead (leading to several changes that all of the same form), Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.org/jdk/pull/21507/files - new: https://git.openjdk.org/jdk/pull/21507/files/fd19351b..e1460f57 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21507&range=23 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21507&range=22-23 Stats: 11 lines in 1 file changed: 5 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/21507.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21507/head:pull/21507 PR: https://git.openjdk.org/jdk/pull/21507
Re: RFR: 8342650: Move getChars to DecimalDigits
On Tue, 12 Nov 2024 01:25:16 GMT, Shaojin Wen wrote: > This PR is a resubmission after PR #21593 was rolled back, and the unsafe > offset overflow issue has been fixed. > > Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to > reduce duplication > > HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16 > > Putting these two methods into DecimalDigits can avoid the need to expose > them in JavaLangAccess > Eliminate duplicate code in BigDecimal > > This PR will improve the performance of Integer/Long.toString and > StringBuilder.append(int/long) scenarios. This is because Unsafe.putByte is > used to eliminate array bounds checks, and of course this elimination is safe. > > In previous versions, in Integer/Long.toString and > StringBuilder.append(int/long) scenarios, -COMPACT_STRING performed better > than +COMPACT_STRING. This is because StringUTF16.getChars uses > StringUTF16.putChar, which is similar to Unsafe.putChar, and there is no > bounds check. @wenshao you need a new JBS issue to complete this work under. - PR Comment: https://git.openjdk.org/jdk/pull/22023#issuecomment-2469426151
Re: RFR: 8343064: ClassFormatError: Illegal class name from InnerClassLambdaMetafactory [v2]
On Tue, 5 Nov 2024 19:21:58 GMT, Chen Liang wrote:
>> After the ClassFile API migration, when serializable lambdas are requested
>> for hidden class callers, illegal class name is generated for the
>> serialization methods, which previously had legal yet unusable class names,
>> as hidden classes cannot be described by a `CONSTANT_Class_info`.
>>
>> This patch restores a similar older behavior of using legal yet unusable
>> class names. Previously, the invalid `.` in hidden class names was replaced
>> by a `/` as if a package separator; this patch changes it to a `_` like that
>> in the generated lambda's class name.
>>
>> The bug report shows some unintended and erroneous usages of
>> `LambdaMetafactory`. To clarify and to persuade against misuse, I added a
>> paragraph of API notes to `LambdaMetafactory`, describing the impact of this
>> API being designed to support Java language features. In particular, I used
>> the scenario where people assumed LMf generates weak hidden classes, which
>> happened in this issue report and in #12493, that misuse can lead to
>> resource leaks.
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add a test to ensure serializable lambda creation and basic execution in
> hidden classes
src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 210:
> 208: * referenced method; no desugaring is needed.)
> 209: *
> 210: * Uses besides evaluation of lambda expressions and method
> references are
I think this paragraph should go at the end (just before the `@since` tag), so
it doesn't interrupt the explanation.
src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 212:
> 210: * Uses besides evaluation of lambda expressions and method
> references are
> 211: * unintended. These linkage methods may change their unspecified
> behaviors at
> 212: * any time to better suit the Java language features, which may impact
Suggestion:
* any time to better suit the Java language features they were designed to
support, which may impact
src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 221:
> 219: * where the caller is {@linkplain
> MethodHandles.Lookup#defineHiddenClass not
> 220: * strongly reachable} from its defining class loader, and such use
> cases may
> 221: * lead to resource leaks.
Not sure about having an example here, I think something more punchy is needed.
IMO we should just specify the behavior, not motivate the design decisions
(which are irrelevant to most users).
Maybe a simple warning such as: 'Unintended uses of these linkage methods may
lead to resource leaks, or other unspecified negative effects.' would be enough.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21912#discussion_r1836411298
PR Review Comment: https://git.openjdk.org/jdk/pull/21912#discussion_r1836403675
PR Review Comment: https://git.openjdk.org/jdk/pull/21912#discussion_r1836396166
Re: RFR: 8340133: Add concise usage message to the java executable [v7]
On Mon, 11 Nov 2024 08:13:18 GMT, Jan Lahoda wrote: >> Currently, running `java` without any parameters will lead to an output that >> is a full `--help`, which is over 100 lines (on my computer at least), and >> it feels overwhelming. And many people might actually want to run >> JShell/REPL, not the `java` executable, but it is difficult find out about >> JShell. >> >> The proposal herein is to print a much shorter help, together with a pointer >> to JShell, when the executable does not know what to do. I.e. there is >> nothing specified to start, and no option like `--help` is specified. In >> particular, on my machine, it prints: >> >> openjdk 24-internal 2025-03-18 >> >> Usage: java [java options...] [application arguments...] >> >> Where is one of: >> to execute the main method of a compiled class >> -jar .jar to execute the main class of a JAR archive >> -m [/] to execute the main class of a module >> .java to compile and execute a source-file program >> >> Where key java options include: >> --class-path >> where is a list of directories and JAR archives to search >> for class files, separated by ":" >> --module-path >> where is a list of directories and JAR archives to search >> for modules, separated by ":" >> >> For additional help on usage: java --help >> For an interactive Java environment:jshell >> >> >> Hopefully, this may be easier both for people trying to run something, and >> for people that are really looking for JShell. >> >> What do you think? >> >> Thanks! > > Jan Lahoda has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 11 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8340133-2 > - Using correct pplaceholders. > - Adjusting text as suggested. > - Cleaning up the concise message: >- using 2 spaces instead of 4, >- rewording the "for more use --help" part of the message as suggested to > avoid the word "launcher". > - Using lowercase for the keys in the help, using 'source-file' program > instead of 'single-file' program. > - Using an enum instead of booleans, as suggested. > - Adjusting the concise help as suggested: 'using main class of a JAR > archive' and '.jar'/'.java' > - Adjusting the concise help based on review suggestions. > - Cleanup. > - Adjusting/improving the concise help. > - ... and 1 more: https://git.openjdk.org/jdk/compare/99abd24b...b4d7b493 Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/21411#pullrequestreview-2426961678
Re: RFR: 8342650: Move getChars to DecimalDigits [v4]
On Sun, 20 Oct 2024 17:33:09 GMT, j3graham wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add benchmark
>
> src/java.base/share/classes/java/math/BigDecimal.java line 4216:
>
>> 4214: // Get the significand as an absolute value
>> 4215: if (intCompact != INFLATED) {
>> 4216: coeff = new char[19];
>
> A possibility here would be to change `coeff` to be a String. The “else”
> branch already creates a string and has to additionally create a char array
> from it. If this is the only place where the `DecimalDigits.getChars(…
> char[])` is used, some extra code duplication could be removed.
@j3graham I have submitted PR #22009
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21593#discussion_r1836545951
Re: RFR: 8343771: Some FFM benchmarks are broken
On Thu, 7 Nov 2024 19:09:46 GMT, Per Minborg wrote: > This PR fixes some regressions in the FFM benchmarks introduced by > https://bugs.openjdk.org/browse/JDK-8332744. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/21963#issuecomment-2467974348
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v7]
> After 8339120, gcc began catching many different instances of unused code in > the Windows specific codebase. Some of these seem to be bugs. I've taken the > effort to mark out all the relevant globals and locals that trigger the > unused warnings and addressed all of them by commenting out the code as > appropriate. I am confident that in many cases this simplistic approach of > commenting out code does not fix the underlying issue, and the warning > actually found a bug that should be fixed. In these instances, I will be > aiming to fix these bugs with help from reviewers, so I recommend anyone > reviewing who knows more about the code than I do to see whether there is > indeed a bug that needs fixing in a different way than what I did Julian Waters 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 eight additional commits since the last revision: - Merge branch 'openjdk:master' into unused - Neater warning silencer in proc_md.h - Revert _WIN32 workaround in log_messages.c - Copyright in VersionInfo.cpp - Remove neutralLangId in VersionInfo.cpp - Remove global memHandle, would've liked to keep it as a comment :( - Merge branch 'master' into unused - 8342682 - Changes: - all: https://git.openjdk.org/jdk/pull/21616/files - new: https://git.openjdk.org/jdk/pull/21616/files/5e9039fb..53036a65 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21616&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21616&range=05-06 Stats: 200965 lines in 1998 files changed: 125072 ins; 52904 del; 22989 mod Patch: https://git.openjdk.org/jdk/pull/21616.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21616/head:pull/21616 PR: https://git.openjdk.org/jdk/pull/21616
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 31 Oct 2024 19:07:42 GMT, Chris Plummer wrote: >>> I do wonder if mutex support can be implemented for Windows with >>> Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it >>> would be nice to have. Shame threads.h is not available with some Visual >>> Studio versions we support, or at all with gcc. mtx_lock/unlock would be a >>> nice solution to this problem >> >> I'll also start looking into this in the meantime, unless you explicitly >> want me not to do so > >> > I do wonder if mutex support can be implemented for Windows with >> > Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it >> > would be nice to have. Shame threads.h is not available with some Visual >> > Studio versions we support, or at all with gcc. mtx_lock/unlock would be a >> > nice solution to this problem >> >> I'll also start looking into this in the meantime, unless you explicitly >> want me not to do so > > Feel free to if you'd like to. Personally I don't consider it to be that > important. Bumping, @plummercj I found that casting to void in MUTEX_LOCK and MUTEX_UNLOCK also works, and it seems neater to me. Is this ok with you? - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2467690492
Re: RFR: 8311302: Implement JEP 493: Linking Run-Time Images without JMODs [v48]
On Fri, 8 Nov 2024 17:07:55 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Update copyright headers Thanks for the reviews! I'll integrate this later today. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2467671430
Re: RFR: 8340133: Add concise usage message to the java executable [v7]
On Mon, 11 Nov 2024 08:13:18 GMT, Jan Lahoda wrote: >> Currently, running `java` without any parameters will lead to an output that >> is a full `--help`, which is over 100 lines (on my computer at least), and >> it feels overwhelming. And many people might actually want to run >> JShell/REPL, not the `java` executable, but it is difficult find out about >> JShell. >> >> The proposal herein is to print a much shorter help, together with a pointer >> to JShell, when the executable does not know what to do. I.e. there is >> nothing specified to start, and no option like `--help` is specified. In >> particular, on my machine, it prints: >> >> openjdk 24-internal 2025-03-18 >> >> Usage: java [java options...] [application arguments...] >> >> Where is one of: >> to execute the main method of a compiled class >> -jar .jar to execute the main class of a JAR archive >> -m [/] to execute the main class of a module >> .java to compile and execute a source-file program >> >> Where key java options include: >> --class-path >> where is a list of directories and JAR archives to search >> for class files, separated by ":" >> --module-path >> where is a list of directories and JAR archives to search >> for modules, separated by ":" >> >> For additional help on usage: java --help >> For an interactive Java environment:jshell >> >> >> Hopefully, this may be easier both for people trying to run something, and >> for people that are really looking for JShell. >> >> What do you think? >> >> Thanks! > > Jan Lahoda has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 11 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8340133-2 > - Using correct pplaceholders. > - Adjusting text as suggested. > - Cleaning up the concise message: >- using 2 spaces instead of 4, >- rewording the "for more use --help" part of the message as suggested to > avoid the word "launcher". > - Using lowercase for the keys in the help, using 'source-file' program > instead of 'single-file' program. > - Using an enum instead of booleans, as suggested. > - Adjusting the concise help as suggested: 'using main class of a JAR > archive' and '.jar'/'.java' > - Adjusting the concise help based on review suggestions. > - Cleanup. > - Adjusting/improving the concise help. > - ... and 1 more: https://git.openjdk.org/jdk/compare/d5806885...b4d7b493 Thank you Jan, this looks good to me. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21411#pullrequestreview-2426819039
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path
On Thu, 7 Nov 2024 13:28:29 GMT, Sergey Chernyshev
wrote:
> Create a new cgroup for memory
>
> ```
> sudo mkdir -p /sys/fs/cgroup/memory/test
> ```
>
> Run the following script
>
> ```
> docker run --tty=true --rm --volume=$JAVA_HOME:/jdk --memory 400m
> ubuntu:latest \
> sh -c "sleep 10 ; /jdk/bin/java -Xlog:os+container=trace -version" | grep
> Memory\ Limit &
> sleep 10
> HOSTPID=$(sudo ps -ef | awk '/container=trace/ && !/docker/ && !/awk/ { print
> $2 }')
> echo $HOSTPID | sudo tee /sys/fs/cgroup/memory/test/cgroup.procs
> sleep 10
> ```
>
> In the above script, a containerized process (`/bin/sh`) is moved to cgroup
> `/test` before `/jdk/bin/java` gets executed. Java inherits cgroup `/test`
> from its parent process, its `_root` will be `/docker/`,
> `cgroup_path` will be `/test`.
OK, but why is https://bugs.openjdk.org/browse/JDK-8322420 not in effect in
such a case?
> The result would be ($JAVA_HOME points to JDK before fix)
>
> ```
> 9804
> [0.001s][trace][os,container] Memory Limit failed: -2
> [0.001s][trace][os,container] Memory Limit failed: -2
> [0.002s][trace][os,container] Memory Limit failed: -2
> [0.043s][trace][os,container] Memory Limit failed: -2
> ```
>
> JDK updated version:
>
> ```
> 10001
> [0.001s][trace ][os,container] Memory Limit is: 419430400
> [0.001s][trace ][os,container] Memory Limit is: 419430400
> [0.002s][trace ][os,container] Memory Limit is: 419430400
> [0.035s][trace ][os,container] Memory Limit is: 419430400
> ```
It would be good to see the full boot JVM output at the trace level. I'm
wondering why the adjustment isn't sufficient for the use-case the bug
describes. I.e. if the move happens *before* the JVM starts then there is a
chance it works OK by detecting some limit. If not it would really be useful to
understand it better.
If, however, the cgroup move happens after the JVM has started, there is
nothing in the JVM which "corrects" the detected physical memory (i.e. heap
size et. al) and/or detected CPUs. It's not supported to do that dynamically.
-
PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2467779326
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v3]
On Mon, 11 Nov 2024 10:06:11 GMT, Severin Gehwolf wrote: > > I didn't check cg v2 because the issue (NPE) was observed in v1 hosts only. > > The JBS issue doesn't mention `NullPointerException`. It would be good to > list the observed NPE issue. I also wonder, then, if the issue is NPE if [JDK-8336881](https://bugs.openjdk.org/browse/JDK-8336881) would fix that issue. The controller adjustment doesn't yet happen on the Java (Metrics) level. Only hotspot so far. - PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2467787891
Re: RFR: 8342650: Move getChars to DecimalDigits [v4]
On Sun, 10 Nov 2024 08:58:18 GMT, Shaojin Wen wrote: >> Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to >> reduce duplication >> >> 1. HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16 >> 2. Putting these two methods into DecimalDigits can avoid the need to expose >> them in JavaLangAccess >> 3. Eliminate duplicate code in BigDecimal > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > add benchmark This caused a regression, see [JDK-8343925](https://bugs.openjdk.org/browse/JDK-8343925). You might want to consider a quick [backout](https://openjdk.org/guide/#backing-out-a-change). - PR Comment: https://git.openjdk.org/jdk/pull/21593#issuecomment-2468016407
Re: RFR: 8342650: Move getChars to DecimalDigits [v4]
On Sun, 10 Nov 2024 08:58:18 GMT, Shaojin Wen wrote: >> Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to >> reduce duplication >> >> 1. HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16 >> 2. Putting these two methods into DecimalDigits can avoid the need to expose >> them in JavaLangAccess >> 3. Eliminate duplicate code in BigDecimal > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > add benchmark I see Shaojin has created an issue to exclude the test but I think better to back this out quickly. - PR Comment: https://git.openjdk.org/jdk/pull/21593#issuecomment-2468030968
RFR: 8343925: Test HugeToString.java crashes at java.util.BitSet.toString()Ljava/lang/String
8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, so submit this PR to roll back - Commit messages: - Revert "8342650: Move getChars to DecimalDigits" Changes: https://git.openjdk.org/jdk/pull/22012/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22012&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8343925 Stats: 757 lines in 12 files changed: 352 ins; 381 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/22012.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22012/head:pull/22012 PR: https://git.openjdk.org/jdk/pull/22012
Re: RFR: 8342650: Move getChars to DecimalDigits [v4]
On Sun, 10 Nov 2024 08:58:18 GMT, Shaojin Wen wrote: >> Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to >> reduce duplication >> >> 1. HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16 >> 2. Putting these two methods into DecimalDigits can avoid the need to expose >> them in JavaLangAccess >> 3. Eliminate duplicate code in BigDecimal > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > add benchmark I submitted a rollback PR #22012. I can't reproduce the problem on a MacBook M1 Max, but I agree that more testing is needed, so let's roll it back first. - PR Comment: https://git.openjdk.org/jdk/pull/21593#issuecomment-2468089941
RFR: 8315585: Optimization for decimal to string
Continue to complete PR #16006 to improve performance and reduce duplicate code - Commit messages: - optimize BigDecimal toString/toPlainString - add benchmark Changes: https://git.openjdk.org/jdk/pull/22009/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22009&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8315585 Stats: 391 lines in 3 files changed: 188 ins; 121 del; 82 mod Patch: https://git.openjdk.org/jdk/pull/22009.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22009/head:pull/22009 PR: https://git.openjdk.org/jdk/pull/22009
Re: RFR: 8342650: Move getChars to DecimalDigits [v4]
On Sun, 10 Nov 2024 08:58:18 GMT, Shaojin Wen wrote: >> Move getChars methods of StringLatin1 and StringUTF16 to DecimalDigits to >> reduce duplication >> >> 1. HexDigits and OctalDigits also include getCharsLatin1 and getCharsUTF16 >> 2. Putting these two methods into DecimalDigits can avoid the need to expose >> them in JavaLangAccess >> 3. Eliminate duplicate code in BigDecimal > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > add benchmark I think the fix is easy: Unsafe calls are using a bad offset which should be cast to long. Will submit a PR, but since I cannot reproduce the original issue locally or on internal CI, need others review first. - PR Comment: https://git.openjdk.org/jdk/pull/21593#issuecomment-2468066742
Integrated: 8311302: Implement JEP 493: Linking Run-Time Images without JMODs
On Thu, 6 Jul 2023 17:34:10 GMT, Severin Gehwolf wrote: > Please review this patch which adds a jlink mode to the JDK which doesn't > need the packaged modules being present. A.k.a run-time image based jlink. > Fundamentally this patch adds an option to use `jlink` even though your JDK > install might not come with the packaged modules (directory `jmods`). This is > particularly useful to further reduce the size of a jlinked runtime. After > the removal of the concept of a JRE, a common distribution mechanism is still > the full JDK with all modules and packaged modules. However, packaged modules > can incur an additional size tax. For example in a container scenario it > could be useful to have a base JDK container including all modules, but > without also delivering the packaged modules. This comes at a size advantage > of `~25%`. Such a base JDK container could then be used to `jlink` > application specific runtimes, further reducing the size of the application > runtime image (App + JDK runtime; as a single image *or* separate bundles, > depending on the app b eing modularized). > > The basic design of this approach is to add a jlink plugin for tracking > non-class and non-resource files of a JDK install. I.e. files which aren't > present in the jimage (`lib/modules`). This enables producing a `JRTArchive` > class which has all the info of what constitutes the final jlinked runtime. > > Basic usage example: > > $ diff -u <(./bin/java --list-modules --limit-modules java.se) > <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules > --limit-modules java.se) > $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) > <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules > --limit-modules jdk.jlink) > $ ls ../linux-x86_64-server-release/images/jdk/jmods > java.base.jmodjava.net.http.jmod java.sql.rowset.jmod > jdk.crypto.ec.jmod jdk.internal.opt.jmod > jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod > java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod > jdk.dynalink.jmod jdk.internal.vm.ci.jmod > jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod > java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod > jdk.editpad.jmod jdk.internal.vm.compiler.jmod > jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod > java.desktop.jmod java.scripting.jmod java.xml.jmod > jdk.hotspot.agent.jmod jdk.internal.vm.compiler.manage... This pull request has now been integrated. Changeset: 2ec35808 Author:Severin Gehwolf URL: https://git.openjdk.org/jdk/commit/2ec358082f0896480bdbfcb289b4ba2bff0dd828 Stats: 4315 lines in 47 files changed: 4065 ins; 146 del; 104 mod 8311302: Implement JEP 493: Linking Run-Time Images without JMODs Co-authored-by: Mandy Chung Reviewed-by: mchung, alanb, erikj, ihse - PR: https://git.openjdk.org/jdk/pull/14787
Question about Streams, Gatherers, and fetching too many elements
Hello Core Libs Dev Team, I was trying out Gatherers for a project at work, and ran into a rather sad scenario. I need to process a large file in batches. Each batch is small enough that I can hold it in memory, but I cannot hold the entire file (and thus, all of the batches) in memory at once. Looking at the Gatherers API, I saw windowFixed and thought that it would be a great match for my use case. However, when trying it out, I was disappointed to see that it ran out of memory very quickly. Here is my attempt at using it. stream .parallel() .unordered() .gather(Gatherers.windowFixed(BATCH_SIZE)) .forEach(eachList -> println(eachList.getFirst())) ; As you can see, I am just splitting the file into batches, and printing out the first of each batch. This is purely for example's sake, of course. I had planned on building even more functionality on top of this, but I couldn't even get past this example. But anyways, not even a single one of them printed out. Which leads me to believe that it's pulling all of them in the Gatherer. I can get it to run successfully if I go sequentially, but not parallel. Parallel gives me that out of memory error. Is there any way for me to be able to have the Gatherer NOT pull in everything while still remaining parallel and unordered? Thank you for your time and help. David Alayachew
RFR: 8343933: Add a MemorySegment::fill benchmark with varying sizes
This PR proposes to add a new `MemorySegment::fill" benchmark where the byte size of the segments varies. - Commit messages: - Use static arrays - Revert changes from other branch - Add benchmark - Add benchmarks Changes: https://git.openjdk.org/jdk/pull/22010/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22010&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8343933 Stats: 150 lines in 1 file changed: 150 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/22010.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22010/head:pull/22010 PR: https://git.openjdk.org/jdk/pull/22010
Re: RFR: 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits
On Mon, 11 Nov 2024 12:34:44 GMT, Shaojin Wen wrote: > 8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, > > I can't reproduce the problem on a MacBook M1 Max, but I agree that more > testing is needed, so let's roll it back first. It has been verified that it is caused by unsafe offset overflow. The problem has been reproduced and fixed. I submitted PR #22014. Would you consider fixing it this way? - PR Comment: https://git.openjdk.org/jdk/pull/22012#issuecomment-2468215673
Re: Question about Streams, Gatherers, and fetching too many elements
And just to avoid the obvious question, I can hold about 30 batches in memory before the Out of Memory error occurs. So this is not an issue of my batch size being too high. But just to confirm, I set the batch size to 1, and it still ran into an out of memory error. So I feel fairly confident saying that the Gatherer is trying to grab all available data before sending any of it downstream. On Mon, Nov 11, 2024, 8:46 AM David Alayachew wrote: > Hello Core Libs Dev Team, > > I was trying out Gatherers for a project at work, and ran into a rather > sad scenario. > > I need to process a large file in batches. Each batch is small enough that > I can hold it in memory, but I cannot hold the entire file (and thus, all > of the batches) in memory at once. > > Looking at the Gatherers API, I saw windowFixed and thought that it would > be a great match for my use case. > > However, when trying it out, I was disappointed to see that it ran out of > memory very quickly. Here is my attempt at using it. > > stream > .parallel() > .unordered() > .gather(Gatherers.windowFixed(BATCH_SIZE)) > .forEach(eachList -> println(eachList.getFirst())) > ; > > As you can see, I am just splitting the file into batches, and printing > out the first of each batch. This is purely for example's sake, of course. > I had planned on building even more functionality on top of this, but I > couldn't even get past this example. > > But anyways, not even a single one of them printed out. Which leads me to > believe that it's pulling all of them in the Gatherer. > > I can get it to run successfully if I go sequentially, but not parallel. > Parallel gives me that out of memory error. > > Is there any way for me to be able to have the Gatherer NOT pull in > everything while still remaining parallel and unordered? > > Thank you for your time and help. > David Alayachew >
Re: RFR: 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits
On Mon, 11 Nov 2024 12:34:44 GMT, Shaojin Wen wrote: > 8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, > > I can't reproduce the problem on a MacBook M1 Max, but I agree that more > testing is needed, so let's roll it back first. Changes in this area need to be very carefully reviewed and tested. I think continue with the current plan to blackout the original change and seeing wider review and testing for the REDO. Chen is testing the blackout now. - PR Comment: https://git.openjdk.org/jdk/pull/22012#issuecomment-2468230634
Re: RFR: 8340133: Add concise usage message to the java executable [v7]
> Currently, running `java` without any parameters will lead to an output that > is a full `--help`, which is over 100 lines (on my computer at least), and it > feels overwhelming. And many people might actually want to run JShell/REPL, > not the `java` executable, but it is difficult find out about JShell. > > The proposal herein is to print a much shorter help, together with a pointer > to JShell, when the executable does not know what to do. I.e. there is > nothing specified to start, and no option like `--help` is specified. In > particular, on my machine, it prints: > > openjdk 24-internal 2025-03-18 > > Usage: java [java options...] [application arguments...] > > Where is one of: > to execute the main method of a compiled class > -jar .jar to execute the main class of a JAR archive > -m [/] to execute the main class of a module > .java to compile and execute a source-file program > > Where key java options include: > --class-path > where is a list of directories and JAR archives to search > for class files, separated by ":" > --module-path > where is a list of directories and JAR archives to search > for modules, separated by ":" > > For additional help on usage: java --help > For an interactive Java environment:jshell > > > Hopefully, this may be easier both for people trying to run something, and > for people that are really looking for JShell. > > What do you think? > > Thanks! Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision: - Merge branch 'master' into JDK-8340133-2 - Using correct pplaceholders. - Adjusting text as suggested. - Cleaning up the concise message: - using 2 spaces instead of 4, - rewording the "for more use --help" part of the message as suggested to avoid the word "launcher". - Using lowercase for the keys in the help, using 'source-file' program instead of 'single-file' program. - Using an enum instead of booleans, as suggested. - Adjusting the concise help as suggested: 'using main class of a JAR archive' and '.jar'/'.java' - Adjusting the concise help based on review suggestions. - Cleanup. - Adjusting/improving the concise help. - ... and 1 more: https://git.openjdk.org/jdk/compare/e60df682...b4d7b493 - Changes: - all: https://git.openjdk.org/jdk/pull/21411/files - new: https://git.openjdk.org/jdk/pull/21411/files/d1d6e4ab..b4d7b493 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21411&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21411&range=05-06 Stats: 295479 lines in 3320 files changed: 187870 ins; 78143 del; 29466 mod Patch: https://git.openjdk.org/jdk/pull/21411.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21411/head:pull/21411 PR: https://git.openjdk.org/jdk/pull/21411
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v3]
On Fri, 8 Nov 2024 16:11:37 GMT, Sergey Chernyshev wrote: > I didn't check cg v2 because the issue (NPE) was observed in v1 hosts only. The JBS issue doesn't mention `NullPointerException`. It would be good to list the observed NPE issue. - PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2467736667
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v3]
On Mon, 11 Nov 2024 15:48:48 GMT, Severin Gehwolf wrote: > So on cg v1 you start out and end with a `subsystem_path() == null` and on cg > v2 you start out and end with a `subsystem_path() == > /../../../../../../test`. In both cases the memory limit of 400m won't be > detected. Only when docker fails to mount the cgroup while moving process to an outer group or a sibling group. It's probably not the case with CloudFoundry. - PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2468791946
Re: RFR: 8342486: Implement JEP draft: Structured Concurrency (Fourth Preview)
On Wed, 6 Nov 2024 15:47:55 GMT, Alan Bateman wrote: > Changes for [JEP draft: Structured Concurrency (Fourth > Preview)](https://openjdk.org/jeps/8340343). The JEP isn't on the technical > roadmap yet. The proposal is to re-preview the API with some changes, > specifically: > > - A > [StructuredTaskScope](https://download.java.net/java/early_access/loom/docs/api/java.base/java/util/concurrent/StructuredTaskScope.html) > is now opened with a static factory method instead of a constructor. Once > opened, the API usage is unchanged: fork subtasks individually, join them as > a unit, process outcome, and close. > - In conjunction with moving to using a static open method, policy and > desired outcome is now selected by specifying a Joiner to the open method > rather than extending STS. A Joiner handles subtask completion and produces > the result for join to return. Joiner.onComplete is the equivalent of > overriding handleComplete previously. This change means that the subclasses > ShutdownOnFailure and ShutdownOnSuccess are removed, replaced by factory > methods on Joiner to get an equivalent Joiner. > - The join method is changed to return the result or throw > STS.FailedException, replacing the need for an API in subclasses to obtain > the outcome. This removes the hazard that was forgetting to call > throwIfFailed to propagate exceptions. > - Configuration that was provided with parameters for the constructor is > changed so that can be provided by a configuration function. > - joinUntil is replaced by allowing a timeout be configured by the > configuration function. This allows the timeout to apply the scope rather > than the join method. > > The underlying implementation is unchanged except that ThreadFlock.shutdown > and wakeup methods are no longer confined. The STS API implementation moves > to non-public StructuedTaskScopeImpl because STS is now an interface. A > non-public Joiners class is added with the built-in Joiner implementations. src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 645: > 643: * > 644: * This Joiner can also be used for fan-in > scenarios where subtasks > 645: * for forked to handle incoming connections and the number of > subtasks is unbounded. Sentence does not read well. Should 'for' have been 'are'? Suggestion: * are forked to handle incoming connections and the number of subtasks is unbounded. - PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r1837091844
Re: [External] : Re: Question about Streams, Gatherers, and fetching too many elements
You're most welcome!
In a potential future where all intermediate operations are Gatherer-based, and
all terminal operations are Collector-based, it would just work as expected.
But with that said, I'm not sure it is practically achievable because some
operations might not have the same performance-characteristics as before.
Cheers,
√
Viktor Klang
Software Architect, Java Platform Group
Oracle
From: David Alayachew
Sent: Monday, 11 November 2024 18:32
To: Viktor Klang
Cc: core-libs-dev
Subject: [External] : Re: Question about Streams, Gatherers, and fetching too
many elements
Thanks for the workaround. It's running beautifully.
Is there a future where this island concept is extended to the rest of streams?
Tbh, I don't fully understand it.
On Mon, Nov 11, 2024, 9:59 AM Viktor Klang
mailto:viktor.kl...@oracle.com>> wrote:
Hi David,
This is the effect of how parallel streams are implemented, where different
stages, which are not representible as a join-less Spliterator are executed as
a series of "islands" where the next isn't started until the former has
completed.
If you think about it, parallelization of a Stream works best when the entire
data set can be split amongst a set of worker threads, and that sort of implies
that you want eager pre-fetch of data, so if your dataset does not fit in
memory, that is likely to lead to less desirable outcomes.
What I was able to do for Gatherers is to implement "gather(…) +
collect(…)"-fusion so any number of consecutive gather(…)-operations
immediately followed by a collect(…) is run in the same "island".
So with that said, you could try something like the following:
static Collector forEach(Consumer each) {
return Collector.of(() -> null, (v, e) -> each.accept(e), (l, r) -> l, (v)
-> null, Collector.Characteristics.IDENTITY_FINISH);
}
stream
.parallel()
.unordered()
.gather(Gatherers.windowFixed(BATCH_SIZE))
.collect(forEach(eachList -> println(eachList.getFirst(;
Cheers,
√
Viktor Klang
Software Architect, Java Platform Group
Oracle
From: core-libs-dev
mailto:core-libs-dev-r...@openjdk.org>> on
behalf of David Alayachew
mailto:davidalayac...@gmail.com>>
Sent: Monday, 11 November 2024 14:52
To: core-libs-dev mailto:core-libs-dev@openjdk.org>>
Subject: Re: Question about Streams, Gatherers, and fetching too many elements
And just to avoid the obvious question, I can hold about 30 batches in memory
before the Out of Memory error occurs. So this is not an issue of my batch size
being too high.
But just to confirm, I set the batch size to 1, and it still ran into an out of
memory error. So I feel fairly confident saying that the Gatherer is trying to
grab all available data before sending any of it downstream.
On Mon, Nov 11, 2024, 8:46 AM David Alayachew
mailto:davidalayac...@gmail.com>> wrote:
Hello Core Libs Dev Team,
I was trying out Gatherers for a project at work, and ran into a rather sad
scenario.
I need to process a large file in batches. Each batch is small enough that I
can hold it in memory, but I cannot hold the entire file (and thus, all of the
batches) in memory at once.
Looking at the Gatherers API, I saw windowFixed and thought that it would be a
great match for my use case.
However, when trying it out, I was disappointed to see that it ran out of
memory very quickly. Here is my attempt at using it.
stream
.parallel()
.unordered()
.gather(Gatherers.windowFixed(BATCH_SIZE))
.forEach(eachList -> println(eachList.getFirst()))
;
As you can see, I am just splitting the file into batches, and printing out the
first of each batch. This is purely for example's sake, of course. I had
planned on building even more functionality on top of this, but I couldn't even
get past this example.
But anyways, not even a single one of them printed out. Which leads me to
believe that it's pulling all of them in the Gatherer.
I can get it to run successfully if I go sequentially, but not parallel.
Parallel gives me that out of memory error.
Is there any way for me to be able to have the Gatherer NOT pull in everything
while still remaining parallel and unordered?
Thank you for your time and help.
David Alayachew
Re: RFR: 8343933: Add a MemorySegment::fill benchmark with varying sizes
On Mon, 11 Nov 2024 13:59:38 GMT, Francesco Nigro wrote: > Thanks @minborg for this :) Please remember to add the misprediction count if > you can and avoid the bulk methods by having a `nextMemorySegment()` > benchmark method which make a single fill call site to observe the different > segments (types). > > Having separate call-sites which observe always the same type(s) "could" be > too lucky (and gentle) for the runtime (and CHA) and would favour to have a > single address entry (or few ones, if we include any optimization for the > fill size) in the Branch Target Buffer of the cpu. I've added a "mixed" benchmark. I am not sure I understood all of your comments but given my changes, maybe you could elaborate a bit more? - PR Comment: https://git.openjdk.org/jdk/pull/22010#issuecomment-2468362180
Re: Question about Streams, Gatherers, and fetching too many elements
Hi David,
This is the effect of how parallel streams are implemented, where different
stages, which are not representible as a join-less Spliterator are executed as
a series of "islands" where the next isn't started until the former has
completed.
If you think about it, parallelization of a Stream works best when the entire
data set can be split amongst a set of worker threads, and that sort of implies
that you want eager pre-fetch of data, so if your dataset does not fit in
memory, that is likely to lead to less desirable outcomes.
What I was able to do for Gatherers is to implement "gather(…) +
collect(…)"-fusion so any number of consecutive gather(…)-operations
immediately followed by a collect(…) is run in the same "island".
So with that said, you could try something like the following:
static Collector forEach(Consumer each) {
return Collector.of(() -> null, (v, e) -> each.accept(e), (l, r) -> l, (v)
-> null, Collector.Characteristics.IDENTITY_FINISH);
}
stream
.parallel()
.unordered()
.gather(Gatherers.windowFixed(BATCH_SIZE))
.collect(forEach(eachList -> println(eachList.getFirst(;
Cheers,
√
Viktor Klang
Software Architect, Java Platform Group
Oracle
From: core-libs-dev on behalf of David
Alayachew
Sent: Monday, 11 November 2024 14:52
To: core-libs-dev
Subject: Re: Question about Streams, Gatherers, and fetching too many elements
And just to avoid the obvious question, I can hold about 30 batches in memory
before the Out of Memory error occurs. So this is not an issue of my batch size
being too high.
But just to confirm, I set the batch size to 1, and it still ran into an out of
memory error. So I feel fairly confident saying that the Gatherer is trying to
grab all available data before sending any of it downstream.
On Mon, Nov 11, 2024, 8:46 AM David Alayachew
mailto:davidalayac...@gmail.com>> wrote:
Hello Core Libs Dev Team,
I was trying out Gatherers for a project at work, and ran into a rather sad
scenario.
I need to process a large file in batches. Each batch is small enough that I
can hold it in memory, but I cannot hold the entire file (and thus, all of the
batches) in memory at once.
Looking at the Gatherers API, I saw windowFixed and thought that it would be a
great match for my use case.
However, when trying it out, I was disappointed to see that it ran out of
memory very quickly. Here is my attempt at using it.
stream
.parallel()
.unordered()
.gather(Gatherers.windowFixed(BATCH_SIZE))
.forEach(eachList -> println(eachList.getFirst()))
;
As you can see, I am just splitting the file into batches, and printing out the
first of each batch. This is purely for example's sake, of course. I had
planned on building even more functionality on top of this, but I couldn't even
get past this example.
But anyways, not even a single one of them printed out. Which leads me to
believe that it's pulling all of them in the Gatherer.
I can get it to run successfully if I go sequentially, but not parallel.
Parallel gives me that out of memory error.
Is there any way for me to be able to have the Gatherer NOT pull in everything
while still remaining parallel and unordered?
Thank you for your time and help.
David Alayachew
Re: RFR: 8343933: Add a MemorySegment::fill benchmark with varying sizes [v2]
On Mon, 11 Nov 2024 14:50:57 GMT, Per Minborg wrote:
>> This PR proposes to add a new `MemorySegment::fill" benchmark where the byte
>> size of the segments varies.
>
> Per Minborg 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 six additional
> commits since the last revision:
>
> - Add mixed test
> - Merge branch 'master' into ms-fill-bench-sizes
> - Use static arrays
> - Revert changes from other branch
> - Add benchmark
> - Add benchmarks
test/micro/org/openjdk/bench/java/lang/foreign/SegmentBulkRandomFill.java line
169:
> 167: public void mixedSegmentFillUnsafe() {
> 168: for (int i = 0; i < INSTANCES; i++) {
> 169: MIXED_SEGMENTS[i].fill((byte) 0);
loop unrolling can defeat a bit the purpose of messing up with branch
misprediction here so...one solution is to avoid loop unrolling OR
create a
@CompilerControl(DONTINLINE)
static void fillSegment(MemorySegment ms, byte value) {
memorySegment.fill(value);
}
which makes sure (at the cost of a very predictable call to the `fillSegment`
method) that:
- the inlining depth of fill is controlled and not dependent by the depth of
the JMH infra caller
- the call site for fill is always the same (in term of the address in the
compiled blob)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/22010#discussion_r1836806492
Re: RFR: 8336707: Contention of ForkJoinPool grows when stealing works [v23]
On Mon, 11 Nov 2024 16:01:08 GMT, Doug Lea wrote: >> This addresses tendencies in previous update to increase fencing, scanning, >> and signalling that can increase contention, and slow down performance >> especially on ARM platforms. It also uses more ARM-friendly constructions to >> reduce overhead (leading to several changes that all of the same form), > > Doug Lea has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge remote-tracking branch 'refs/remotes/origin/JDK-8336707' into > JDK-8336707 > - Reconcile internal docs; renamings src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 417: > 415: * * STOP: no more tasks run, and deregister all workers > 416: * * CLEANED: all unexecuted tasks have been cancelled > 417: * * TERMINATED: all qorkers deregistered and all queues cleaned Suggestion: * * TERMINATED: all workers deregistered and all queues cleaned src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 663: > 661: * * A call to shutdownNow, in which case all workers are > 662: * interrupted. ensuring that the queues array is stable, > 663: * so will not miss any of them. Suggestion: * interrupted, and ensuring that the array of queues is stable, * so will not miss any of them. src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 676: > 674: * cancel (benefitting from parallelism) versus contention and > 675: * interference when many threads try to poll remaining queues, > 676: * while also avoiding unnecessary rechedcks, by using Suggestion: * while also avoiding unnecessary rechecks, by using src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 688: > 686: * before executing task bodies, and ensures interrupts while > 687: * terminating. Even so, there are no guarantees because tasks may > 688: * internally enter unbounded loops. Good point to make src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 874: > 872: * cancellation by others, which can occur along several different > 873: * paths. The inability to rely on caller-runs may also require > 874: * extra signalling (and resulting scanning and contention) so is Suggestion: * extra signalling (resulting in increased scanning and contention) so is - PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836899379 PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836903791 PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836908015 PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836908858 PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836910984
Re: RFR: 8343933: Add a MemorySegment::fill benchmark with varying sizes
On Mon, 11 Nov 2024 11:55:27 GMT, Per Minborg wrote: > This PR proposes to add a new `MemorySegment::fill" benchmark where the byte > size of the segments varies. Thanks @minborg for this :) Please remember to add the misprediction count if you can - or just avoid the bulk methods instead i.e. having a `nextMemorySegment()` benchmark method which make the fill to observe the different segments. Having separate call-sites which observe always the same type(s) "could" be too lucky (and gentle) for the runtime (and its CHA) - PR Comment: https://git.openjdk.org/jdk/pull/22010#issuecomment-2468247252
Re: RFR: 8340133: Add concise usage message to the java executable [v7]
On Mon, 11 Nov 2024 08:13:18 GMT, Jan Lahoda wrote: >> Currently, running `java` without any parameters will lead to an output that >> is a full `--help`, which is over 100 lines (on my computer at least), and >> it feels overwhelming. And many people might actually want to run >> JShell/REPL, not the `java` executable, but it is difficult find out about >> JShell. >> >> The proposal herein is to print a much shorter help, together with a pointer >> to JShell, when the executable does not know what to do. I.e. there is >> nothing specified to start, and no option like `--help` is specified. In >> particular, on my machine, it prints: >> >> openjdk 24-internal 2025-03-18 >> >> Usage: java [java options...] [application arguments...] >> >> Where is one of: >> to execute the main method of a compiled class >> -jar .jar to execute the main class of a JAR archive >> -m [/] to execute the main class of a module >> .java to compile and execute a source-file program >> >> Where key java options include: >> --class-path >> where is a list of directories and JAR archives to search >> for class files, separated by ":" >> --module-path >> where is a list of directories and JAR archives to search >> for modules, separated by ":" >> >> For additional help on usage: java --help >> For an interactive Java environment:jshell >> >> >> Hopefully, this may be easier both for people trying to run something, and >> for people that are really looking for JShell. >> >> What do you think? >> >> Thanks! > > Jan Lahoda has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 11 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8340133-2 > - Using correct pplaceholders. > - Adjusting text as suggested. > - Cleaning up the concise message: >- using 2 spaces instead of 4, >- rewording the "for more use --help" part of the message as suggested to > avoid the word "launcher". > - Using lowercase for the keys in the help, using 'source-file' program > instead of 'single-file' program. > - Using an enum instead of booleans, as suggested. > - Adjusting the concise help as suggested: 'using main class of a JAR > archive' and '.jar'/'.java' > - Adjusting the concise help based on review suggestions. > - Cleanup. > - Adjusting/improving the concise help. > - ... and 1 more: https://git.openjdk.org/jdk/compare/7f4880aa...b4d7b493 src/java.base/share/classes/sun/launcher/resources/launcher.properties line 241: > 239: \ -jar .jar to execute the main class of a JAR > archive\n\ > 240: \ -m [/] to execute the main class of a module\n\ > 241: \ .java to compile and execute a source-file > program\n\n\ I'm not sure about the description of . It uses "compiled class", maybe you means "compiled main class" or something else to connect it ? "-jar .jar" may be confusing because the "java -help" uses "-java ". I think the usages need to be the same. - PR Review Comment: https://git.openjdk.org/jdk/pull/21411#discussion_r1836748117
Re: RFR: 8336707: Contention of ForkJoinPool grows when stealing works [v21]
On Sun, 10 Nov 2024 17:51:43 GMT, Doug Lea wrote: >> This addresses tendencies in previous update to increase fencing, scanning, >> and signalling that can increase contention, and slow down performance >> especially on ARM platforms. It also uses more ARM-friendly constructions to >> reduce overhead (leading to several changes that all of the same form), > > Doug Lea 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 40 additional commits since > the last revision: > > - Merge branch 'openjdk:master' into JDK-8336707 > - Minor improvements > - Merge branch 'openjdk:master' into JDK-8336707 > - Add CLEANED runState > - Merge branch 'openjdk:master' into JDK-8336707 > - More shutdown streamlining > - Don't report termination while cancellations in progress > - Reduce read contention > - Merge branch 'openjdk:master' into JDK-8336707 > - Reduce shutdown overhead and contention > - ... and 30 more: https://git.openjdk.org/jdk/compare/c6713e65...4db28137 src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 414: > 412: * roles, as lockable, versioned counters. Field runState also > 413: * includes monotonic event bits (SHUTDOWN, STOP, CLEANED (when > 414: * all queues are knowm to be empty after stopping) and Suggestion: * all queues are known to be empty after stopping) and src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 662: > 660: * tasks until runState is marked as CLEANED (staggering queues > 661: * and backing off on interference to avoid contention while doing > 662: * so-- see method cleanQueues). These actions race with Suggestion: * so--see method cleanQueues). These actions race with - PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836824631 PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1836828143
Integrated: 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits
On Mon, 11 Nov 2024 12:34:44 GMT, Shaojin Wen wrote: > 8343925 Feedback PR #21593 test/jdk/java/util/BitSet/HugeToString.java crash, > > I can't reproduce the problem on a MacBook M1 Max, but I agree that more > testing is needed, so let's roll it back first. This pull request has now been integrated. Changeset: b0a371b0 Author:Shaojin Wen URL: https://git.openjdk.org/jdk/commit/b0a371b0850b8f467ed985ef39a6fce476b62acf Stats: 757 lines in 12 files changed: 352 ins; 381 del; 24 mod 8343925: [BACKOUT] JDK-8342650 Move getChars to DecimalDigits Reviewed-by: jpai, alanb, liach - PR: https://git.openjdk.org/jdk/pull/22012
Re: RFR: 8334048: -Xbootclasspath can not read some ZIP64 zip files [v4]
On Tue, 10 Sep 2024 05:46:46 GMT, Jaikiran Pai wrote: >> I pushed the review suggestions. There were two GitHub Actions failures on >> `macos-aarch64`, but they look to be occurrences of this existing bug: >> https://bugs.openjdk.org/browse/JDK-8247940. > > Hello @fitzsim, I plan to take a look at this change and also consult with > others familiar with the bootclasspath area as well as jar/zip area. I am > just noting this now to let you know that the PR hasn't been neglected and I > think it will take a while for this to be reviewed given the nature and area > of this change. > Sounds good @jaikiran. > > I can provide some background about this pull request; it was the result of > discussions on a related jdk8u-dev pull request > ([openjdk/jdk8u-dev#452](https://github.com/openjdk/jdk8u-dev/pull/452)). > > Red Hat has been shipping the `zip_util.c` change proposed here, authored by > @gnu-andrew, as a patch to our Red Hat Enterprise Linux `java-1.8.0-openjdk` > packages since mid-2020. In that release all `ZIP` files the `JDK` uses are > processed by `zip_util.c` unless someone is using the demo `Java` > implementation. We have had no field reports of issues with the patch. > > @gnu-andrew and I would like to upstream `ZIP64` support to `jdk8u`, but the > `zip_util.c` patch only exists within Red Hat's package repository, not > anywhere upstream from where it could be backported. > > When I checked for the most recent branch that still shipped `zip_util.c`, I > discovered that even mainline still ships it, albeit now only for use in > `-Xbootclasspath` handling. So I split the `zip_util.c` patch out and added > some tests to demonstrate the bug itself and that there should be no > regressions, and filed this pull request. > > If this is accepted into mainline and gets some broader testing with no > issue, then I would like to backport it to 21, 17, 11, and 8. Just to add to this, while the `zip_util.c` changes are unique to our 8u patch, they are a conversion to C of the equivalent Java changes made in [JDK-8186464](https://bugs.openjdk.org/browse/JDK-8186464), with the intention to backport that change to 8u without the risk of [JDK-8145260](https://bugs.openjdk.org/browse/JDK-8145260), which introduces the Java-based implementation. So if there is a risk in this patch, it is likely a risk of a mistake in the translation to C rather than the general gist of the changes, which are present in the JDK's Java zip code. - PR Comment: https://git.openjdk.org/jdk/pull/19678#issuecomment-2468614066
Re: RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path
On Mon, 11 Nov 2024 15:16:15 GMT, Severin Gehwolf wrote: > On cg v2, on the other hand, `set_subsystem_path()` will never set the path > to a `null` value. Exactly. That's why JDK-8322420 is not in effect and also [JDK-8336881](https://bugs.openjdk.org/browse/JDK-8336881) does not fix it on Java side (path stays uninitialized in certain conditions). - PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2468614420
Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]
On Tue, 12 Nov 2024 02:59:59 GMT, Patricio Chilano Mateo wrote: >> This is the implementation of JEP 491: Synchronize Virtual Threads without >> Pinning. See [JEP 491](https://bugs.openjdk.org/browse/JDK-8337395) for >> further details. >> >> In order to make the code review easier the changes have been split into the >> following initial 4 commits: >> >> - Changes to allow unmounting a virtual thread that is currently holding >> monitors. >> - Changes to allow unmounting a virtual thread blocked on synchronized >> trying to acquire the monitor. >> - Changes to allow unmounting a virtual thread blocked in `Object.wait()` >> and its timed-wait variants. >> - Changes to tests, JFR pinned event, and other changes in the JDK libraries. >> >> The changes fix pinning issues for all 4 ports that currently implement >> continuations: x64, aarch64, riscv and ppc. Note: ppc changes were added >> recently and stand in its own commit after the initial ones. >> >> The changes fix pinning issues when using `LM_LIGHTWEIGHT`, i.e. the default >> locking mode, (and `LM_MONITOR` which comes for free), but not when using >> `LM_LEGACY` mode. Note that the `LockingMode` flag has already been >> deprecated ([JDK-8334299](https://bugs.openjdk.org/browse/JDK-8334299)), >> with the intention to remove `LM_LEGACY` code in future releases. >> >> >> ## Summary of changes >> >> ### Unmount virtual thread while holding monitors >> >> As stated in the JEP, currently when a virtual thread enters a synchronized >> method or block, the JVM records the virtual thread's carrier platform >> thread as holding the monitor, not the virtual thread itself. This prevents >> the virtual thread from being unmounted from its carrier, as ownership >> information would otherwise go wrong. In order to fix this limitation we >> will do two things: >> >> - We copy the oops stored in the LockStack of the carrier to the stackChunk >> when freezing (and clear the LockStack). We copy the oops back to the >> LockStack of the next carrier when thawing for the first time (and clear >> them from the stackChunk). Note that we currently assume carriers don't hold >> monitors while mounting virtual threads. >> >> - For inflated monitors we now record the `java.lang.Thread.tid` of the >> owner in the ObjectMonitor's `_owner` field instead of a JavaThread*. This >> allows us to tie the owner of the monitor to a `java.lang.Thread` instance, >> rather than to a JavaThread which is only created per platform thread. The >> tid is already a 64 bit field so we can ignore issues of the counter >> wrapping around. >> >> General notes about this part: >> >> - Since virtual th... > > Patricio Chilano Mateo has updated the pull request with a new target base > due to a merge or a rebase. The pull request now contains 90 commits: > > - Merge branch 'master' into JDK-8338383 > - Test StopThreadTest.java: fix operator in condition + improve names > - Pass -XX:-UseCompactObjectHeaders in test JNIMonitor.java > - Merge branch 'master' into JDK-8338383 > - Fix in JvmtiEnvBase::get_locked_objects_in_frame() > - Add ObjectWaiter::at_monitorenter > - Use get_method_name + copyright revert in jvmtiThreadState.cpp > - Merge branch 'master' into JDK-8338383 > - Add @requires vm.continuations to CancelTimerWithContention.java > - Use JvmtiVTMSTransitionDisabler::VTMS_vthread_mount/unmount > - ... and 80 more: https://git.openjdk.org/jdk/compare/babb52a0...0fe60465 Still good. Thanks - Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21565#pullrequestreview-2428723863
RFR: 8343984: Fix Unsafe address overflow
In the JDK code, there are some places that may cause Unsafe offset overflow. The probability of occurrence is low, but if it occurs, it will cause JVM crash. - Commit messages: - fix unsafe address overflow Changes: https://git.openjdk.org/jdk/pull/22027/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22027&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8343984 Stats: 17 lines in 9 files changed: 0 ins; 0 del; 17 mod Patch: https://git.openjdk.org/jdk/pull/22027.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22027/head:pull/22027 PR: https://git.openjdk.org/jdk/pull/22027
Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]
> This is the implementation of JEP 491: Synchronize Virtual Threads without > Pinning. See [JEP 491](https://bugs.openjdk.org/browse/JDK-8337395) for > further details. > > In order to make the code review easier the changes have been split into the > following initial 4 commits: > > - Changes to allow unmounting a virtual thread that is currently holding > monitors. > - Changes to allow unmounting a virtual thread blocked on synchronized trying > to acquire the monitor. > - Changes to allow unmounting a virtual thread blocked in `Object.wait()` and > its timed-wait variants. > - Changes to tests, JFR pinned event, and other changes in the JDK libraries. > > The changes fix pinning issues for all 4 ports that currently implement > continuations: x64, aarch64, riscv and ppc. Note: ppc changes were added > recently and stand in its own commit after the initial ones. > > The changes fix pinning issues when using `LM_LIGHTWEIGHT`, i.e. the default > locking mode, (and `LM_MONITOR` which comes for free), but not when using > `LM_LEGACY` mode. Note that the `LockingMode` flag has already been > deprecated ([JDK-8334299](https://bugs.openjdk.org/browse/JDK-8334299)), with > the intention to remove `LM_LEGACY` code in future releases. > > > ## Summary of changes > > ### Unmount virtual thread while holding monitors > > As stated in the JEP, currently when a virtual thread enters a synchronized > method or block, the JVM records the virtual thread's carrier platform thread > as holding the monitor, not the virtual thread itself. This prevents the > virtual thread from being unmounted from its carrier, as ownership > information would otherwise go wrong. In order to fix this limitation we will > do two things: > > - We copy the oops stored in the LockStack of the carrier to the stackChunk > when freezing (and clear the LockStack). We copy the oops back to the > LockStack of the next carrier when thawing for the first time (and clear them > from the stackChunk). Note that we currently assume carriers don't hold > monitors while mounting virtual threads. > > - For inflated monitors we now record the `java.lang.Thread.tid` of the owner > in the ObjectMonitor's `_owner` field instead of a JavaThread*. This allows > us to tie the owner of the monitor to a `java.lang.Thread` instance, rather > than to a JavaThread which is only created per platform thread. The tid is > already a 64 bit field so we can ignore issues of the counter wrapping around. > > General notes about this part: > > - Since virtual threads don't need to worry about holding monitors anymo... Patricio Chilano Mateo has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 90 commits: - Merge branch 'master' into JDK-8338383 - Test StopThreadTest.java: fix operator in condition + improve names - Pass -XX:-UseCompactObjectHeaders in test JNIMonitor.java - Merge branch 'master' into JDK-8338383 - Fix in JvmtiEnvBase::get_locked_objects_in_frame() - Add ObjectWaiter::at_monitorenter - Use get_method_name + copyright revert in jvmtiThreadState.cpp - Merge branch 'master' into JDK-8338383 - Add @requires vm.continuations to CancelTimerWithContention.java - Use JvmtiVTMSTransitionDisabler::VTMS_vthread_mount/unmount - ... and 80 more: https://git.openjdk.org/jdk/compare/babb52a0...0fe60465 - Changes: https://git.openjdk.org/jdk/pull/21565/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21565&range=04 Stats: 9984 lines in 249 files changed: 7169 ins; 1629 del; 1186 mod Patch: https://git.openjdk.org/jdk/pull/21565.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21565/head:pull/21565 PR: https://git.openjdk.org/jdk/pull/21565
Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]
On Tue, 12 Nov 2024 02:59:59 GMT, Patricio Chilano Mateo wrote: >> This is the implementation of JEP 491: Synchronize Virtual Threads without >> Pinning. See [JEP 491](https://bugs.openjdk.org/browse/JDK-8337395) for >> further details. >> >> In order to make the code review easier the changes have been split into the >> following initial 4 commits: >> >> - Changes to allow unmounting a virtual thread that is currently holding >> monitors. >> - Changes to allow unmounting a virtual thread blocked on synchronized >> trying to acquire the monitor. >> - Changes to allow unmounting a virtual thread blocked in `Object.wait()` >> and its timed-wait variants. >> - Changes to tests, JFR pinned event, and other changes in the JDK libraries. >> >> The changes fix pinning issues for all 4 ports that currently implement >> continuations: x64, aarch64, riscv and ppc. Note: ppc changes were added >> recently and stand in its own commit after the initial ones. >> >> The changes fix pinning issues when using `LM_LIGHTWEIGHT`, i.e. the default >> locking mode, (and `LM_MONITOR` which comes for free), but not when using >> `LM_LEGACY` mode. Note that the `LockingMode` flag has already been >> deprecated ([JDK-8334299](https://bugs.openjdk.org/browse/JDK-8334299)), >> with the intention to remove `LM_LEGACY` code in future releases. >> >> >> ## Summary of changes >> >> ### Unmount virtual thread while holding monitors >> >> As stated in the JEP, currently when a virtual thread enters a synchronized >> method or block, the JVM records the virtual thread's carrier platform >> thread as holding the monitor, not the virtual thread itself. This prevents >> the virtual thread from being unmounted from its carrier, as ownership >> information would otherwise go wrong. In order to fix this limitation we >> will do two things: >> >> - We copy the oops stored in the LockStack of the carrier to the stackChunk >> when freezing (and clear the LockStack). We copy the oops back to the >> LockStack of the next carrier when thawing for the first time (and clear >> them from the stackChunk). Note that we currently assume carriers don't hold >> monitors while mounting virtual threads. >> >> - For inflated monitors we now record the `java.lang.Thread.tid` of the >> owner in the ObjectMonitor's `_owner` field instead of a JavaThread*. This >> allows us to tie the owner of the monitor to a `java.lang.Thread` instance, >> rather than to a JavaThread which is only created per platform thread. The >> tid is already a 64 bit field so we can ignore issues of the counter >> wrapping around. >> >> General notes about this part: >> >> - Since virtual th... > > Patricio Chilano Mateo has updated the pull request with a new target base > due to a merge or a rebase. The pull request now contains 90 commits: > > - Merge branch 'master' into JDK-8338383 > - Test StopThreadTest.java: fix operator in condition + improve names > - Pass -XX:-UseCompactObjectHeaders in test JNIMonitor.java > - Merge branch 'master' into JDK-8338383 > - Fix in JvmtiEnvBase::get_locked_objects_in_frame() > - Add ObjectWaiter::at_monitorenter > - Use get_method_name + copyright revert in jvmtiThreadState.cpp > - Merge branch 'master' into JDK-8338383 > - Add @requires vm.continuations to CancelTimerWithContention.java > - Use JvmtiVTMSTransitionDisabler::VTMS_vthread_mount/unmount > - ... and 80 more: https://git.openjdk.org/jdk/compare/babb52a0...0fe60465 I merged with master, including the changes for [JEP 450](https://github.com/openjdk/jdk/pull/20677), and run it through tiers1-8 in mach5 with no related failures. I would like to integrate tomorrow if I could get some re-approvals. Also, I filed JDK-8343957 to possibly improve the naming of `_lock_id/owner_from` as discussed in some of the comments. - PR Comment: https://git.openjdk.org/jdk/pull/21565#issuecomment-2469493942
Re: RFR: 8341260: Add Float16 to jdk.incubator.vector [v11]
On Mon, 11 Nov 2024 14:03:54 GMT, Raffaello Giulietti
wrote:
>> Joe Darcy has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Respond to review feedback.
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java line
> 431:
>
>> 429: * @seejava.lang.Float#valueOf(String)
>> 430: */
>> 431: public static Float16 valueOf(String s) throws
>> NumberFormatException {
>
> The current implementation throws when the input is `"NaN"`, with or without
> an optional sign.
Thanks for catching that; fix and tests added in subsequent push.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21574#discussion_r1837508472
Re: RFR: 8341260: Add Float16 to jdk.incubator.vector [v12]
> Port of Float16 from java.lang in the lworld+fp16 branch to > jdk.incubabor.vector. Joe Darcy 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: - Test update and minor bug fix. - Merge branch 'master' into JDK-8341260 - Add Float16 -> string conversion from Raffaello. - Merge branch 'master' into JDK-8341260 - Merge branch 'master' into JDK-8341260 - Respond to review feedback. - Add support for proper String -> Float16 conversion. - Merge branch 'master' into JDK-8341260 - Improve specification per code review feedback. - Add tests, improve hashing spec. - ... and 11 more: https://git.openjdk.org/jdk/compare/f69b3a3a...8af04c4d - Changes: - all: https://git.openjdk.org/jdk/pull/21574/files - new: https://git.openjdk.org/jdk/pull/21574/files/7c50bbe0..8af04c4d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21574&range=11 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21574&range=10-11 Stats: 205741 lines in 2051 files changed: 129905 ins; 52738 del; 23098 mod Patch: https://git.openjdk.org/jdk/pull/21574.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21574/head:pull/21574 PR: https://git.openjdk.org/jdk/pull/21574
Re: RFR: 8341260: Add Float16 to jdk.incubator.vector [v12]
On Tue, 12 Nov 2024 05:42:46 GMT, Joe Darcy wrote: >> Port of Float16 from java.lang in the lworld+fp16 branch to >> jdk.incubabor.vector. > > Joe Darcy 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: > > - Test update and minor bug fix. > - Merge branch 'master' into JDK-8341260 > - Add Float16 -> string conversion from Raffaello. > - Merge branch 'master' into JDK-8341260 > - Merge branch 'master' into JDK-8341260 > - Respond to review feedback. > - Add support for proper String -> Float16 conversion. > - Merge branch 'master' into JDK-8341260 > - Improve specification per code review feedback. > - Add tests, improve hashing spec. > - ... and 11 more: https://git.openjdk.org/jdk/compare/aaa007cb...8af04c4d Updated the implementation with Float16 -> string conversion code from @rgiulietti and added various supporting tests. - PR Comment: https://git.openjdk.org/jdk/pull/21574#issuecomment-2469662058
