Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v4]

[Jaikiran Pai]

On Mon, 11 Jul 2022 15:31:46 GMT, Roger Riggs  wrote:

>> The `ProcessBuilder.pipelineStart()` implementation does not close all of 
>> the file descriptors it uses to create the pipeline of processes.
>> 
>> The process calling `pipelineStart()` is creating the pipes between the 
>> stages.
>> As each process is launched, the file descriptor is inherited by the child 
>> process and
>> the child process `dup`s them to the respective stdin/stdout/stderr fd.  
>> These copies of inherited file descriptors are handled correctly.
>> 
>> Between the launching of each Process, the file descriptor for the read-side 
>> of the pipe for the output of the previous process is kept open (in the 
>> parent process invoking `pipelineStart`).  The file descriptor is correctly 
>> passed to the child and is dup'd to the stdin of the next process.
>> 
>> However, the open file descriptor in the parent is not closed after it has 
>> been used as the input for the next Process. 
>> The fix is to close the fd after it has been used as the input of the next 
>> process.
>> 
>> A new test verifies that after `pipelineStart` is complete, the same file 
>> descriptors are open for Unix Pipes as before the test.
>> The test only runs on Linux using the /proc//fd filesystem to identify 
>> open file descriptors.
>> 
>> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all 
>> platforms.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove debug output

Hello Roger, I just noticed that the failures reported in the GitHub actions 
job are genuine and related to this new test. 2 separate failures:


test 
PipelineLeaksFD.checkForLeaks(java.util.ImmutableCollections$List12@73c167a8): 
failure
java.lang.AssertionError: More or fewer pipes than expected
at org.testng.Assert.fail(Assert.java:99)
at PipelineLeaksFD.checkForLeaks(PipelineLeaksFD.java:94)
at 
java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:578)
at 
org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
at 
org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
at 
org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
at 
org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
at 
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at org.testng.TestRunner.privateRun(TestRunner.java:764)
at org.testng.TestRunner.run(TestRunner.java:585)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
at org.testng.SuiteRunner.run(SuiteRunner.java:286)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
at org.testng.TestNG.runSuites(TestNG.java:1069)
at org.testng.TestNG.run(TestNG.java:1037)
at 
com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
at 
com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54)
at 
java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:578)
at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
at java.base/java.lang.Thread.run(Thread.java:1589)


and



test 
PipelineLeaksFD.checkForLeaks(java.util.ImmutableCollections$ListN@13c9b949): 
failure
java.io.UncheckedIOException: java.nio.file.NoSuchFileException: 
/proc/18459/fd/20
at 
java.base/java.nio.file.FileTreeIterator.fetchNextIfNeeded(FileTreeIterator.java:87)
at 
java.base/java.nio.file.FileTreeIterator.hasNext(FileTreeIterator.java:103)
at java.base/java.util.Iterator.forEachRemaining(Iterator.java:132)
at 
java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1921)
at 
java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at 
java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at 

Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v4]

[Jaikiran Pai]

On Mon, 11 Jul 2022 15:31:46 GMT, Roger Riggs  wrote:

>> The `ProcessBuilder.pipelineStart()` implementation does not close all of 
>> the file descriptors it uses to create the pipeline of processes.
>> 
>> The process calling `pipelineStart()` is creating the pipes between the 
>> stages.
>> As each process is launched, the file descriptor is inherited by the child 
>> process and
>> the child process `dup`s them to the respective stdin/stdout/stderr fd.  
>> These copies of inherited file descriptors are handled correctly.
>> 
>> Between the launching of each Process, the file descriptor for the read-side 
>> of the pipe for the output of the previous process is kept open (in the 
>> parent process invoking `pipelineStart`).  The file descriptor is correctly 
>> passed to the child and is dup'd to the stdin of the next process.
>> 
>> However, the open file descriptor in the parent is not closed after it has 
>> been used as the input for the next Process. 
>> The fix is to close the fd after it has been used as the input of the next 
>> process.
>> 
>> A new test verifies that after `pipelineStart` is complete, the same file 
>> descriptors are open for Unix Pipes as before the test.
>> The test only runs on Linux using the /proc//fd filesystem to identify 
>> open file descriptors.
>> 
>> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all 
>> platforms.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove debug output

Marked as reviewed by jpai (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/9414

Re: [jdk19] RFR: 8288850: SegmentAllocator:allocate() can return null some cases

[Paul Sandoz]

On Mon, 11 Jul 2022 15:05:34 GMT, Maurizio Cimadamore  
wrote:

> This patch fixes an issue where the arena allocator does not detect an 
> overflow when attempting to allocate a new slice.

Marked as reviewed by psandoz (Reviewer).

-

PR: https://git.openjdk.org/jdk19/pull/133

Re: [jdk19] RFR: 8289365: SegmentAllocator:allocateArray(MemoryLayout, count) does not throw IAEx when count is -1

[Paul Sandoz]

On Mon, 11 Jul 2022 14:45:42 GMT, Maurizio Cimadamore  
wrote:

> The default implementation for `SegmentAllocator:allocateArray(MemoryLayout, 
> count)` is missing a check for negative array size.
> In the past this used to be caught in the `SequenceLayout` factory, but not 
> anymore, as now `SequenceLayout` accepts `-1` as special size where the 
> sequence element count is inferred. So, an explicit check is needed.

Marked as reviewed by psandoz (Reviewer).

-

PR: https://git.openjdk.org/jdk19/pull/132

Re: [jdk19] RFR: 8290071: Javadoc for MemorySegment/MemoryAddress getter/setters contains some typos

[Paul Sandoz]

On Mon, 11 Jul 2022 09:46:11 GMT, Maurizio Cimadamore  
wrote:

> Some of the accessors in `MemorySegment` and `MemoryAddress` (esp. setters) 
> have typos - e.g. they use `from` instead of `into`.
> 
> I've tried to simplify the text and made it more regular.

Marked as reviewed by psandoz (Reviewer).

-

PR: https://git.openjdk.org/jdk19/pull/131

Re: RFR: JDK-8289106: Add model of class file versions to core reflection [v2]

[Mandy Chung]

On Wed, 29 Jun 2022 21:32:29 GMT, Joe Darcy  wrote:

>> JDK-8289106: Add model of class file versions to core reflection
>
> 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 10 additional commits since 
> the last revision:
> 
>  - Add method to map from major class file version.
>  - Merge branch 'master' into JDK-8289106
>  - Update phrasing.
>  - Augment and correct docs.
>  - Correct major versions; RELEASE_0 and RELEASE_1 are both 45.
>  - Make major method public.
>  - Add AccessFlag.locations(ClassFileFormatVersion)
>  - Add ClassFileFormatVersion <-> Runtime.Version conversions.
>  - Merge branch 'master' into JDK-8289106
>  - JDK-8289106: Add model of class file versions to core reflection

This `ClassFileFormatVersion` API is a good proposal.   It can also reference 
`java.class.version` system property to map to the latest class file format 
version.

-

PR: https://git.openjdk.org/jdk/pull/9299

[jdk19] Integrated: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

[Christoph Langer]

On Mon, 11 Jul 2022 15:07:28 GMT, Christoph Langer  wrote:

> Hi all,
> 
> This pull request contains a backport of 
> [JDK-8287902](https://bugs.openjdk.org/browse/JDK-8287902), commit 
> [975316e3](https://github.com/openjdk/jdk/commit/975316e3e5f1208e4e15eadc2493d25c15554647)
>  from the [openjdk/jdk](https://git.openjdk.org/jdk) repository.
> 
> The commit being backported was authored by Magnus Ihse Bursie on 10 Jun 2022 
> and was reviewed by Naoto Sato.
> 
> It fixes an issue that shows up in the new GHA workflow.
> 
> Thanks!

This pull request has now been integrated.

Changeset: 39715f3d
Author:Christoph Langer 
URL:   
https://git.openjdk.org/jdk19/commit/39715f3da7e8749bf477b818ae06f4dd99c223c4
Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod

8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably 
on Windows

Backport-of: 975316e3e5f1208e4e15eadc2493d25c15554647

-

PR: https://git.openjdk.org/jdk19/pull/134

Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v4]

[Alan Bateman]

On Mon, 11 Jul 2022 15:31:46 GMT, Roger Riggs  wrote:

>> The `ProcessBuilder.pipelineStart()` implementation does not close all of 
>> the file descriptors it uses to create the pipeline of processes.
>> 
>> The process calling `pipelineStart()` is creating the pipes between the 
>> stages.
>> As each process is launched, the file descriptor is inherited by the child 
>> process and
>> the child process `dup`s them to the respective stdin/stdout/stderr fd.  
>> These copies of inherited file descriptors are handled correctly.
>> 
>> Between the launching of each Process, the file descriptor for the read-side 
>> of the pipe for the output of the previous process is kept open (in the 
>> parent process invoking `pipelineStart`).  The file descriptor is correctly 
>> passed to the child and is dup'd to the stdin of the next process.
>> 
>> However, the open file descriptor in the parent is not closed after it has 
>> been used as the input for the next Process. 
>> The fix is to close the fd after it has been used as the input of the next 
>> process.
>> 
>> A new test verifies that after `pipelineStart` is complete, the same file 
>> descriptors are open for Unix Pipes as before the test.
>> The test only runs on Linux using the /proc//fd filesystem to identify 
>> open file descriptors.
>> 
>> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all 
>> platforms.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove debug output

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/9414

Integrated: Merge jdk19

[Jesper Wilhelmsson]

On Mon, 11 Jul 2022 12:38:11 GMT, Jesper Wilhelmsson  
wrote:

> Forwardport JDK 19 -> JDK 20

This pull request has now been integrated.

Changeset: c79baaa8
Author:Jesper Wilhelmsson 
URL:   
https://git.openjdk.org/jdk/commit/c79baaa811971c43fbdbc251482d0e40903588cc
Stats: 411 lines in 43 files changed: 284 ins; 26 del; 101 mod

Merge

-

PR: https://git.openjdk.org/jdk/pull/9450

Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v4]

[Roger Riggs]

> The `ProcessBuilder.pipelineStart()` implementation does not close all of the 
> file descriptors it uses to create the pipeline of processes.
> 
> The process calling `pipelineStart()` is creating the pipes between the 
> stages.
> As each process is launched, the file descriptor is inherited by the child 
> process and
> the child process `dup`s them to the respective stdin/stdout/stderr fd.  
> These copies of inherited file descriptors are handled correctly.
> 
> Between the launching of each Process, the file descriptor for the read-side 
> of the pipe for the output of the previous process is kept open (in the 
> parent process invoking `pipelineStart`).  The file descriptor is correctly 
> passed to the child and is dup'd to the stdin of the next process.
> 
> However, the open file descriptor in the parent is not closed after it has 
> been used as the input for the next Process. 
> The fix is to close the fd after it has been used as the input of the next 
> process.
> 
> A new test verifies that after `pipelineStart` is complete, the same file 
> descriptors are open for Unix Pipes as before the test.
> The test only runs on Linux using the /proc//fd filesystem to identify 
> open file descriptors.
> 
> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all 
> platforms.

Roger Riggs has updated the pull request incrementally with one additional 
commit since the last revision:

  remove debug output

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9414/files
  - new: https://git.openjdk.org/jdk/pull/9414/files/845695d8..f34a3a09

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9414=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=9414=02-03

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/9414.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9414/head:pull/9414

PR: https://git.openjdk.org/jdk/pull/9414

[jdk19] RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

[Christoph Langer]

Hi all,

This pull request contains a backport of 
[JDK-8287902](https://bugs.openjdk.org/browse/JDK-8287902), commit 
[975316e3](https://github.com/openjdk/jdk/commit/975316e3e5f1208e4e15eadc2493d25c15554647)
 from the [openjdk/jdk](https://git.openjdk.org/jdk) repository.

The commit being backported was authored by Magnus Ihse Bursie on 10 Jun 2022 
and was reviewed by Naoto Sato.

It fixes an issue that shows up in the new GHA workflow.

Thanks!

-

Commit messages:
 - Backport 975316e3e5f1208e4e15eadc2493d25c15554647

Changes: https://git.openjdk.org/jdk19/pull/134/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=134=00
  Issue: https://bugs.openjdk.org/browse/JDK-8287902
  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk19/pull/134.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/134/head:pull/134

PR: https://git.openjdk.org/jdk19/pull/134

[jdk19] Withdrawn: 8288697: Clarify lifecycle of buffer segments and loader lookup

[Maurizio Cimadamore]

On Fri, 17 Jun 2022 21:39:16 GMT, Maurizio Cimadamore  
wrote:

> This is a dependent PR containing a cleanup of the so called *heap sessions*. 
> Heap sessions are used in two cases:
> 
> * when a buffer segment is created, to keep the original buffer instance 
> reachable
> * when a loader symbol lookup is created, to keep the classloader instance 
> reachable
> 
> Spinning new sessions in these cases seems unnecessary, and inconsistent with 
> what we do for segments backed by on-heap arrays, whose session is set to the 
> global session. In that case, it's up to the segment to keep the underlying 
> array reachable. I think that's a better model.
> 
> This patch adds the ability to attach Object references to native and mapped 
> memory segments, so that we can attach loader/byte buffer instances to these 
> segments, keeping them alive. This means that, in these cases we can go back 
> to just use the global memory session, like we do for array segments.
> 
> This simplifies the implementation, makes the documentation more consistent, 
> and also simplifies the user model, as it removes a concept (of heap session) 
> that is not really documented anywhere. In fact, the javadoc for 
> MemorySegment::ofBuffer claims, (wrongly!) that the session of the resulting 
> segment is the global session - but that's not the case.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.org/jdk19/pull/39

Re: [jdk19] RFR: 8289148: j.l.foreign.VaList::nextVarg call could throw IndexOutOfBoundsException or even crash the VM [v4]

[Jorn Vernee]

> This patch changes all VaList implementations to throw 
> `NoSuchElementException` when out of bounds reads occur on a VaList that is 
> created using the Java builder API. The docs are updated accordingly.
> 
> For VaLists that are created from native addresses, we don't know their 
> bounds, so we can not throw exceptions in that case.
> 
> Testing: `jdk_foreign` test suite on all platforms that implement VaList.

Jorn Vernee has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains six commits:

 - Merge branch 'master' into VaList
 - hmtl -> html
 - Review comments
 - Update Javadoc
 - Cleanup
 - Test passing

-

Changes: https://git.openjdk.org/jdk19/pull/91/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=91=03
  Stats: 394 lines in 7 files changed: 237 ins; 19 del; 138 mod
  Patch: https://git.openjdk.org/jdk19/pull/91.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/91/head:pull/91

PR: https://git.openjdk.org/jdk19/pull/91

[jdk19] RFR: 8288850: SegmentAllocator:allocate() can return null some cases

[Maurizio Cimadamore]

This patch fixes an issue where the arena allocator does not detect an overflow 
when attempting to allocate a new slice.

-

Commit messages:
 - Initial push

Changes: https://git.openjdk.org/jdk19/pull/133/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=133=00
  Issue: https://bugs.openjdk.org/browse/JDK-8288850
  Stats: 9 lines in 2 files changed: 8 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk19/pull/133.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/133/head:pull/133

PR: https://git.openjdk.org/jdk19/pull/133

Re: [JMH results for IndexedLinkedList]

[Jens Lideström]

I think you are coming at this in the wrong way, Rodion. I am not a JDK 
contributor, but these are my thoughts on how this works.

The people that are responsible for the Java collections framework are well 
aware of the existence of alternative data structures such as finger trees. 
There are also a lot of various implementations of them available on the 
internet.

The code for these data structures is not the reason that they have not been 
added to Java. The reason is that adding more data structures makes the  
standard library harder to learn for users and increases the maintenance burden 
for JDK developers. The benefits of the new data structure must outweigh these 
costs.

If you want to move forward with this I think you should create a JEP-style 
document with the following content:

* A description of the problem you think should be solved and why the current 
collection framework does not solve it.
* A list of concrete use cases for your contribution.
* An overview of the existing collection framework where you demonstrate how 
your collection would fill a hole.
* An overview of alternative data structures and implementations, where you 
argue that your collection solves the problem in the best way.

Adding a new data structure the the collections framework is a major change. 
There is a very small change that it will be done and it will not be done 
without careful evaluation. The code and the benchmarks is only a small part of 
that work.

BR,
Jens Lideström

On 2022-06-21 13:20, Rodion Efremov wrote:

Hi,

Data structure: IndexedLinkedList 

Benchmark: IndexedLinkedListBenchmark 


Benchmark output: https://github.com/coderodde/indexedLinkedList/#benchmark-output 


 From the benchmark output, we can judge that IndexedLinkedList outperforms 
both java.util.LinkedList and the Apache Commons Collections TreeList. It, 
however, does not seem to supersede the java.util.ArrayList.

Basically, I would expect the IndexedLinkedList to beat the ArrayList where we 
have a lot of following operations:

  * addFirst(E)
  * add(int, E)
  * remove(int)

So, what do you think? I am getting anywhere with that?

Best regards,
rodde

Re: RFR: 8066859: java/lang/ref/OOMEInReferenceHandler.java failed with java.lang.Exception: Reference Handler thread died

[Doug Lea]

On Mon, 11 Jul 2022 04:10:49 GMT, David Holmes  wrote:

>> 8066859 : java/lang/ref/OOMEInReferenceHandler.java failed with 
>> java.lang.Exception: Reference Handler thread died
>
> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
>  line 296:
> 
>> 294: byte spins = 0, postSpins = 0;   // retries upon unpark of 
>> first thread
>> 295: boolean interrupted = false, first = false;
>> 296: Node pred = null, t; // predecessor of node when 
>> enqueued
> 
> Nit: please don't use this style of multi-variable declaration as they are 
> very easy to mis-read. Also the comment only applies to one of the variables.

OK, changed.

> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java
>  line 1626:
> 
>> 1624: }
>> 1625: if (!isHeldExclusively() || !release(savedState = 
>> getState()))
>> 1626: throw LockSupport.staticIllegalMonitorStateException; 
>> // OOM
> 
> How is it possible to get IMSE this deep into the code? And the comment is 
> confusing - OOM?

Clarified to:
// fall through if encountered OutOfMemoryError
if (!isHeldExclusively() || !release(savedState = getState()))
throw LockSupport.staticIllegalMonitorStateException;

-

PR: https://git.openjdk.org/jdk/pull/9427

Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v2]

[Roger Riggs]

On Sat, 9 Jul 2022 02:43:55 GMT, David Schlosnagle  wrote:

>> Roger Riggs has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Cleanup of PipelineLeaksFD test improving error messages and source 
>> cleanup.
>
> test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 130:
> 
>> 128: })
>> 129: .filter(p1 -> p1.link().toString().startsWith("pipe:"))
>> 130: .collect(Collectors.toSet());
> 
> Is this intentionally leaking the returned `DirectoryStream` from 
> `Files.walk` to  trigger the previous failure mode or should this be 
> auto-closed (i.e. https://errorprone.info/bugpattern/StreamResourceLeak )?
> 
> Suggestion:
> 
> try (Stream s = Files.walk(path)) {
> return s.filter(Files::isSymbolicLink)
> .map(p -> {
> try {
> return new PipeRecord(p, 
> Files.readSymbolicLink(p));
> } catch (IOException ioe) {
> }
> return new PipeRecord(p, null);
> })
> .filter(p1 -> p1.link().toString().startsWith("pipe:"))
> .collect(Collectors.toSet());
> }

@schlosna An oversight,  thanks for the reminder.

-

PR: https://git.openjdk.org/jdk/pull/9414

Re: RFR: 8066859: java/lang/ref/OOMEInReferenceHandler.java failed with java.lang.Exception: Reference Handler thread died

[David Holmes]

On Mon, 11 Jul 2022 04:16:44 GMT, David Holmes  wrote:

>> 8066859 : java/lang/ref/OOMEInReferenceHandler.java failed with 
>> java.lang.Exception: Reference Handler thread died
>
> src/java.base/share/classes/java/util/concurrent/locks/LockSupport.java line 
> 463:
> 
>> 461:  *  Preallocated exceptions thrown if acquiring or releasing locks
>> 462:  *  when OutOfMemory.
>> 463:  */
> 
> I don't see why this should be necessary. IMSE is thrown before any state 
> changes occur and so it is is fine if the IMSE is replaced by OOME.
> 
> Even IE should be safe at the points it is thrown.
> 
> Also in both cases you want to see a full and proper stacktrace.

Consider this another way, any place you have a `throw x` it must be safe to 
propagate that exception. It doesn't matter if that is actually `x` or an OOME 
caused by creating `x`.

-

PR: https://git.openjdk.org/jdk/pull/9427

Re: RFR: 8066859: java/lang/ref/OOMEInReferenceHandler.java failed with java.lang.Exception: Reference Handler thread died

[David Holmes]

On Fri, 8 Jul 2022 11:44:53 GMT, Doug Lea  wrote:

> 8066859 : java/lang/ref/OOMEInReferenceHandler.java failed with 
> java.lang.Exception: Reference Handler thread died

src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
 line 296:

> 294: byte spins = 0, postSpins = 0;   // retries upon unpark of first 
> thread
> 295: boolean interrupted = false, first = false;
> 296: Node pred = null, t; // predecessor of node when 
> enqueued

Nit: please don't use this style of multi-variable declaration as they are very 
easy to mis-read. Also the comment only applies to one of the variables.

src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java
 line 1626:

> 1624: }
> 1625: if (!isHeldExclusively() || !release(savedState = 
> getState()))
> 1626: throw LockSupport.staticIllegalMonitorStateException; 
> // OOM

How is it possible to get IMSE this deep into the code? And the comment is 
confusing - OOM?

src/java.base/share/classes/java/util/concurrent/locks/LockSupport.java line 
463:

> 461:  *  Preallocated exceptions thrown if acquiring or releasing locks
> 462:  *  when OutOfMemory.
> 463:  */

I don't see why this should be necessary. IMSE is thrown before any state 
changes occur and so it is is fine if the IMSE is replaced by OOME.

Even IE should be safe at the points it is thrown.

Also in both cases you want to see a full and proper stacktrace.

-

PR: https://git.openjdk.org/jdk/pull/9427

Re: RFR: 8066859: java/lang/ref/OOMEInReferenceHandler.java failed with java.lang.Exception: Reference Handler thread died

[Alan Bateman]

On Fri, 8 Jul 2022 22:42:28 GMT, Doug Lea  wrote:

> I keep trying but failing to change commit message to something jcheck likes?

There should be an "Edit" button in the top right that allows you to edit the 
title and change it to "8066859 : java/lang/ref/OOMEInReferenceHandler.java 
failed ...".   There are two Skara commands that may be useful here. 
Adding a comment "/issue JDK-8066859" will link the PR to this JBS issue. There 
is also a "/summary" command to edit the commit message.

-

PR: https://git.openjdk.org/jdk/pull/9427

Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v3]

[Roger Riggs]

> The `ProcessBuilder.pipelineStart()` implementation does not close all of the 
> file descriptors it uses to create the pipeline of processes.
> 
> The process calling `pipelineStart()` is creating the pipes between the 
> stages.
> As each process is launched, the file descriptor is inherited by the child 
> process and
> the child process `dup`s them to the respective stdin/stdout/stderr fd.  
> These copies of inherited file descriptors are handled correctly.
> 
> Between the launching of each Process, the file descriptor for the read-side 
> of the pipe for the output of the previous process is kept open (in the 
> parent process invoking `pipelineStart`).  The file descriptor is correctly 
> passed to the child and is dup'd to the stdin of the next process.
> 
> However, the open file descriptor in the parent is not closed after it has 
> been used as the input for the next Process. 
> The fix is to close the fd after it has been used as the input of the next 
> process.
> 
> A new test verifies that after `pipelineStart` is complete, the same file 
> descriptors are open for Unix Pipes as before the test.
> The test only runs on Linux using the /proc//fd filesystem to identify 
> open file descriptors.
> 
> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all 
> platforms.

Roger Riggs has updated the pull request incrementally with one additional 
commit since the last revision:

  Add TWR for Files.walk of /proc

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9414/files
  - new: https://git.openjdk.org/jdk/pull/9414/files/0d628d66..845695d8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9414=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=9414=01-02

  Stats: 13 lines in 1 file changed: 1 ins; 0 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/9414.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9414/head:pull/9414

PR: https://git.openjdk.org/jdk/pull/9414

RFR: 8066859: java/lang/ref/OOMEInReferenceHandler.java failed with java.lang.Exception: Reference Handler thread died

[Doug Lea]

8066859 : java/lang/ref/OOMEInReferenceHandler.java failed with 
java.lang.Exception: Reference Handler thread died

-

Commit messages:
 - Also update Xcomp problem list
 - Merge branch 'master' of https://git.openjdk.org/jdk into JDK-8066859
 - Address review comments
 - Disable filling in stack trace for preallocated exceptions
 - Merge branch 'master' into JDK-8066859
 - Test synch under OOME
 - Avoid sync failures when encountering OutOfMemoryErrors
 - Accommodate OutOfMemoryErrors while locking

Changes: https://git.openjdk.org/jdk/pull/9427/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=9427=00
  Issue: https://bugs.openjdk.org/browse/JDK-8066859
  Stats: 418 lines in 6 files changed: 322 ins; 12 del; 84 mod
  Patch: https://git.openjdk.org/jdk/pull/9427.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9427/head:pull/9427

PR: https://git.openjdk.org/jdk/pull/9427

Re: RFR: 8066859: java/lang/ref/OOMEInReferenceHandler.java failed with java.lang.Exception: Reference Handler thread died

[Doug Lea]

On Fri, 8 Jul 2022 11:44:53 GMT, Doug Lea  wrote:

> 8066859 : java/lang/ref/OOMEInReferenceHandler.java failed with 
> java.lang.Exception: Reference Handler thread died

Thanks to @AlanBateman for suggesting to disable possibly misleading stack 
traces in pre-allocated exceptions; now updated

I keep trying but failing to change commit message to something jcheck likes?

The idea here is that a non-OOME-throwing condition wait might be required for 
(some) memory to be reclaimed. This is not too farfetched for 
InterruptedException (if serving as a cancellation), so we allow it to be 
thrown (instead of OOME) in hopes that a catch of IE will free memory. It IS 
farfetched to think this might be the case for IllegalMonitorStateException, 
but I did it the same way for consistency.

-

PR: https://git.openjdk.org/jdk/pull/9427

[jdk19] RFR: 8289365: SegmentAllocator:allocateArray(MemoryLayout, count) does not throw IAEx when count is -1

[Maurizio Cimadamore]

The default implementation for `SegmentAllocator:allocateArray(MemoryLayout, 
count)` is missing a check for negative array size.
In the past this used to be caught in the `SequenceLayout` factory, but not 
anymore, as now `SequenceLayout` accepts `-1` as special size where the 
sequence element count is inferred. So, an explicit check is needed.

-

Commit messages:
 - Initial push

Changes: https://git.openjdk.org/jdk19/pull/132/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=132=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289365
  Stats: 8 lines in 2 files changed: 8 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk19/pull/132.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/132/head:pull/132

PR: https://git.openjdk.org/jdk19/pull/132

Re: [jdk19] RFR: 8288697: Clarify lifecycle of buffer segments and loader lookup [v2]

[Maurizio Cimadamore]

> This is a dependent PR containing a cleanup of the so called *heap sessions*. 
> Heap sessions are used in two cases:
> 
> * when a buffer segment is created, to keep the original buffer instance 
> reachable
> * when a loader symbol lookup is created, to keep the classloader instance 
> reachable
> 
> Spinning new sessions in these cases seems unnecessary, and inconsistent with 
> what we do for segments backed by on-heap arrays, whose session is set to the 
> global session. In that case, it's up to the segment to keep the underlying 
> array reachable. I think that's a better model.
> 
> This patch adds the ability to attach Object references to native and mapped 
> memory segments, so that we can attach loader/byte buffer instances to these 
> segments, keeping them alive. This means that, in these cases we can go back 
> to just use the global memory session, like we do for array segments.
> 
> This simplifies the implementation, makes the documentation more consistent, 
> and also simplifies the user model, as it removes a concept (of heap session) 
> that is not really documented anywhere. In fact, the javadoc for 
> MemorySegment::ofBuffer claims, (wrongly!) that the session of the resulting 
> segment is the global session - but that's not the case.

Maurizio Cimadamore has updated the pull request with a new target base due to 
a merge or a rebase. The pull request now contains eight commits:

 - Initial push
 - Revert implicit vs. heap session changes
 - Unify heap vs. implicit scopes
 - Merge branch 'master' into memory_session_cleanup
 - Fix issue in Direct-X-Buffer template
 - Simplify and drop the state class
 - Add missing files
 - Initial push

-

Changes: https://git.openjdk.org/jdk19/pull/39/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=39=01
  Stats: 577 lines in 28 files changed: 53 ins; 125 del; 399 mod
  Patch: https://git.openjdk.org/jdk19/pull/39.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/39/head:pull/39

PR: https://git.openjdk.org/jdk19/pull/39

[jdk19] Integrated: 8287809: Revisit implementation of memory session

[Maurizio Cimadamore]

On Wed, 15 Jun 2022 18:06:44 GMT, Maurizio Cimadamore  
wrote:

> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017

This pull request has now been integrated.

Changeset: fed3af8a
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.org/jdk19/commit/fed3af8ae069fc760a24e750292acbb468b14ce5
Stats: 429 lines in 21 files changed: 47 ins; 102 del; 280 mod

8287809: Revisit implementation of memory session

Reviewed-by: jvernee

-

PR: https://git.openjdk.org/jdk19/pull/22

RFR: 8290079: Reduce interaction with volatile in static initializer of BigInteger

[Сергей Цыпанов]

`BigInteger.powerCache` is volatile and should be assigned only once in static 
initializer.

-

Commit messages:
 - 8290079: Reduce interaction with volatile in static initializer of BigInteger

Changes: https://git.openjdk.org/jdk/pull/9451/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=9451=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290079
  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/9451.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9451/head:pull/9451

PR: https://git.openjdk.org/jdk/pull/9451

RFR: Merge jdk19

[Jesper Wilhelmsson]

Forwardport JDK 19 -> JDK 20

-

Commit messages:
 - Merge
 - 8290004: [PPC64] JfrGetCallTrace: assert(_pc != nullptr) failed: must have PC
 - 8289692: JFR: Thread checkpoint no longer enforce mutual exclusion post Loom 
integration
 - 8289894: A NullPointerException thrown from guard expression
 - 8289729: G1: Incorrect verification logic in 
G1ConcurrentMark::clear_next_bitmap
 - 8282071: Update java.xml module-info
 - 8290033: ProblemList 
serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java on 
windows-x64 in -Xcomp mode
 - 8289697: buffer overflow in MTLVertexCache.m: MTLVertexCache_AddGlyphQuad
 - 8289872: wrong wording in @param doc for HashMap.newHashMap et. al.
 - 8289601: SegmentAllocator::allocateUtf8String(String str) should be 
clarified for strings containing \0
 - ... and 5 more: https://git.openjdk.org/jdk/compare/46251bc6...0b0d186f

The webrevs contain the adjustments done while merging with regards to each 
parent branch:
 - master: https://webrevs.openjdk.org/?repo=jdk=9450=00.0
 - jdk19: https://webrevs.openjdk.org/?repo=jdk=9450=00.1

Changes: https://git.openjdk.org/jdk/pull/9450/files
  Stats: 411 lines in 43 files changed: 284 ins; 26 del; 101 mod
  Patch: https://git.openjdk.org/jdk/pull/9450.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9450/head:pull/9450

PR: https://git.openjdk.org/jdk/pull/9450

Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[] [v3]

[Сергей Цыпанов]

On Mon, 11 Jul 2022 08:02:43 GMT, Сергей Цыпанов  wrote:

>> The new constructor looks very odd, especially when it does not have an 
>> explanation and doesn't describe the required preconditions for calling it.  
>> Is there a better way than adding a non-functional argument?
>> The "unused" name is going to draw a warning from IntelliJ and some 
>> enterprising developer is going to try to remove it, not understanding why 
>> its there. And there is a bit of overhead pushing the extra arg.
>> 
>> The constructor should be private, it has a very narrow scope of use given 
>> the lack of checking its args.
>> 
>> It would be nice if the Hotspot compiler would recognize the llmits on the 
>> range and optimize away the checks;
>> it would have broader applicability then this point fix.
>> I would be interesting to ask the compiler folks if that's a future 
>> optimization.
>> These source changes make it harder to understand what's happening where; 
>> though that is sometimes work it for a noticeable performance improvement.
>
> Well, we already have e.g. `String(char[], int, int, Void)` and 
> `String(AbstractStringBuilder asb, Void sig)` where trailing argument is for 
> disambiguation against private constructors. I did the same for mine and 
> applied the same naming as in other trailing `Void` params.

Benchmark results after:

Benchmark  Mode  Cnt  Score   
Error  Units
StringConstructor.newStringFromArray   avgt   50  4,354 ± 
0,195  ns/op
StringConstructor.newStringFromArrayWithCharsetavgt   50  4,035 ± 
0,088  ns/op
StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,166 ± 
0,062  ns/op
StringConstructor.newStringFromRangedArray avgt   50  4,132 ± 
0,054  ns/op
StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,416 ± 
0,206  ns/op
StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  7,421 ± 
0,041  ns/op

-

PR: https://git.openjdk.org/jdk/pull/9407

Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[] [v3]

[Сергей Цыпанов]

> We can skip bounds check and null check for Charset in case we use the array 
> entirely and the Charset is either default one or proven to be non-null.
> 
> Benchmark results:
> 
> before
> 
> Benchmark  Mode  Cnt  Score   
> Error  Units
> StringConstructor.newStringFromArray   avgt   50  4,815 ± 
> 0,154  ns/op
> StringConstructor.newStringFromArrayWithCharsetavgt   50  4,462 ± 
> 0,068  ns/op
> StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,653 ± 
> 0,040  ns/op
> StringConstructor.newStringFromRangedArray avgt   50  5,090 ± 
> 0,066  ns/op
> StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,550 ± 
> 0,041  ns/op
> StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  8,080 ± 
> 0,055  ns/op
> 
> after
> 
> Benchmark  Mode  Cnt  Score   
> Error  Units
> StringConstructor.newStringFromArray   avgt   50  4,595 ± 
> 0,053  ns/op
> StringConstructor.newStringFromArrayWithCharsetavgt   50  4,038 ± 
> 0,062  ns/op
> StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,035 ± 
> 0,031  ns/op
> StringConstructor.newStringFromRangedArray avgt   50  4,084 ± 
> 0,007  ns/op
> StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,014 ± 
> 0,008  ns/op
> StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  7,466 ± 
> 0,071  ns/op

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8289908: Fixed tests

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9407/files
  - new: https://git.openjdk.org/jdk/pull/9407/files/294e91f7..b7375cd3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9407=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=9407=01-02

  Stats: 6 lines in 1 file changed: 1 ins; 0 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/9407.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9407/head:pull/9407

PR: https://git.openjdk.org/jdk/pull/9407

Re: JMH results for IndexedLinkedList

[Maurizio Cimadamore]

ConcurrentLinkedDeque was tested and it has similar thoughput to what we 
use, slightly higher memory footprint per element added, so we opted 
against it (but might re-eval in the future).


In the specific case of the FFM API, it is not uncommon to have a 
session with just 1-2 resources attached to it. So having a data 
structure backed by an array becomes a bit problematic, while data 
structures where you pay for what you use are instead preferrable.


We only scan the contents of the data strtucture once (when we bring 
down the session), to call the various cleanup actions, so we're 
absolutely not interested in random access.


Maurizio

On 11/07/2022 11:22, John Hendrikx wrote:


I'm curious, why isn't ArrayDeque or ConcurrentLinkedDeque used 
instead? Or is there another requirement?


ArrayDeque has amortized O(1) for inserts at head and tail (and faster 
and more memory efficient than LinkedList as it doesn't use nodes).


ConcurrentLinkedDeque would be useful in the face of multiple threads 
(it uses nodes though, so won't be as fast as ArrayDeque).


--John

On 11/07/2022 11:58, Maurizio Cimadamore wrote:


The implementation of the Foreign Function & Memory API uses an 
internal custom linked list to add native resources to a "memory 
session" abstraction (things that need to be cleaned up at a later 
point).


Linked list is quite critical in our use case because we need 
something that has a very fast insertion (in the head), and can scale 
gracefully to handle multiple threads.


In our case LinkedList is not good enough (because we want to deal 
with concurrent writes ourselves) - but aside from that, note that, 
at least looking at the numbers posted in your benchmarks, it seems 
that prepending an element to a classic LinkedList is 10x faster than 
ArrayList and 5x faster IndexList. Perhaps that's a case where 
IndexList has not been fully optimized - but for prepend-heavy code 
(and the javac compiler is another one of those), I think performance 
of addFirst is the number to look at.


As Tagir said, of course these use cases are very "niche" - and, at 
least in my experience, deevelopers in this "niche" tend to come up 
with ad-hoc specialized data structures anyways. So the return of 
investment for adding another collection type in this space seems 
relatively low.


Maurizio

On 09/07/2022 20:33, Tagir Valeev wrote:
Note that nobody these days cares about LinkedList. Use-cases where 
LinkedList outperforms careful use of ArrayList or ArrayDeque are 
next to none. So saying that your data structure is better than 
LinkedList is totally not a reason to add it to JDK. It should be 
better than ArrayList and ArrayDeque.


Having a single data structure that provides list and deque 
interface is a reasonable idea. However it would be much simpler to 
retrofit existing data structure like ArrayDeque, rather than create 
a new data structure. Here's an issue for this:

https://bugs.openjdk.org/browse/JDK-8143850

There were also discussions to enhance collections in general, 
adding more useful methods like getFirst() or removeLast() to 
ArrayList, etc. See for details:

https://bugs.openjdk.org/browse/JDK-8266572

To conclude, the idea of adding one more collection implementation 
looks questionable to me. It will add more confusion when people 
need to select which collection fits their needs better. It will 
require more learning. This could be justified if there are clear 
benefits in using it in real world problems, compared to existing 
collections. But so far I don't see the examples of such problems.


With best regards,
Tagir Valeev

сб, 9 июл. 2022 г., 11:22 Rodion Efremov :

Hello,

My benchmarking suggests, that, if nothing else, my
IndexedLinkedList outperforms gracefully the
java.util.LinkedList, so the use case should be the same
(List + Deque -interfaces) for both of the aforementioned
data structures.

Best regards,
rodde


On Sat, Jul 9, 2022 at 11:19 AM Tagir Valeev 
wrote:

Hello!

Are there real world problems/use cases where
IndexedLinkedList would be preferred in terms of CPU/memory
usage over ArrayList?

сб, 9 июл. 2022 г., 07:18 Rodion Efremov :

Data structure repo:
https://github.com/coderodde/IndexedLinkedList

Benchmark repo:
https://github.com/coderodde/IndexedLinkedListBenchmark

I have profiled my data structure and it seems it’s more
performant than java.util.LinkedList or TreeList, if
nothing else.

So, is there any chance of including IndexedLinkedList
to JDK?

Best regards,
rodde

Re: JMH results for IndexedLinkedList

[John Hendrikx]

I'm curious, why isn't ArrayDeque or ConcurrentLinkedDeque used instead? 
Or is there another requirement?


ArrayDeque has amortized O(1) for inserts at head and tail (and faster 
and more memory efficient than LinkedList as it doesn't use nodes).


ConcurrentLinkedDeque would be useful in the face of multiple threads 
(it uses nodes though, so won't be as fast as ArrayDeque).


--John

On 11/07/2022 11:58, Maurizio Cimadamore wrote:


The implementation of the Foreign Function & Memory API uses an 
internal custom linked list to add native resources to a "memory 
session" abstraction (things that need to be cleaned up at a later point).


Linked list is quite critical in our use case because we need 
something that has a very fast insertion (in the head), and can scale 
gracefully to handle multiple threads.


In our case LinkedList is not good enough (because we want to deal 
with concurrent writes ourselves) - but aside from that, note that, at 
least looking at the numbers posted in your benchmarks, it seems that 
prepending an element to a classic LinkedList is 10x faster than 
ArrayList and 5x faster IndexList. Perhaps that's a case where 
IndexList has not been fully optimized - but for prepend-heavy code 
(and the javac compiler is another one of those), I think performance 
of addFirst is the number to look at.


As Tagir said, of course these use cases are very "niche" - and, at 
least in my experience, deevelopers in this "niche" tend to come up 
with ad-hoc specialized data structures anyways. So the return of 
investment for adding another collection type in this space seems 
relatively low.


Maurizio

On 09/07/2022 20:33, Tagir Valeev wrote:
Note that nobody these days cares about LinkedList. Use-cases where 
LinkedList outperforms careful use of ArrayList or ArrayDeque are 
next to none. So saying that your data structure is better than 
LinkedList is totally not a reason to add it to JDK. It should be 
better than ArrayList and ArrayDeque.


Having a single data structure that provides list and deque interface 
is a reasonable idea. However it would be much simpler to retrofit 
existing data structure like ArrayDeque, rather than create a new 
data structure. Here's an issue for this:

https://bugs.openjdk.org/browse/JDK-8143850

There were also discussions to enhance collections in general, adding 
more useful methods like getFirst() or removeLast() to ArrayList, 
etc. See for details:

https://bugs.openjdk.org/browse/JDK-8266572

To conclude, the idea of adding one more collection implementation 
looks questionable to me. It will add more confusion when people need 
to select which collection fits their needs better. It will require 
more learning. This could be justified if there are clear benefits in 
using it in real world problems, compared to existing collections. 
But so far I don't see the examples of such problems.


With best regards,
Tagir Valeev

сб, 9 июл. 2022 г., 11:22 Rodion Efremov :

Hello,

My benchmarking suggests, that, if nothing else, my
IndexedLinkedList outperforms gracefully the
java.util.LinkedList, so the use case should be the same (List
+ Deque -interfaces) for both of the aforementioned data
structures.

Best regards,
rodde


On Sat, Jul 9, 2022 at 11:19 AM Tagir Valeev 
wrote:

Hello!

Are there real world problems/use cases where
IndexedLinkedList would be preferred in terms of CPU/memory
usage over ArrayList?

сб, 9 июл. 2022 г., 07:18 Rodion Efremov :

Data structure repo:
https://github.com/coderodde/IndexedLinkedList

Benchmark repo:
https://github.com/coderodde/IndexedLinkedListBenchmark

I have profiled my data structure and it seems it’s more
performant than java.util.LinkedList or TreeList, if
nothing else.

So, is there any chance of including IndexedLinkedList to
JDK?

Best regards,
rodde

Re: [jdk19] RFR: 8290071: Javadoc for MemorySegment/MemoryAddress getter/setters contains some typos

[Uwe Schindler]

On Mon, 11 Jul 2022 09:46:11 GMT, Maurizio Cimadamore  
wrote:

> Some of the accessors in `MemorySegment` and `MemoryAddress` (esp. setters) 
> have typos - e.g. they use `from` instead of `into`.
> 
> I've tried to simplify the text and made it more regular.

Marked as reviewed by uschindler (Author).

-

PR: https://git.openjdk.org/jdk19/pull/131

Re: JMH results for IndexedLinkedList

[Maurizio Cimadamore]

The implementation of the Foreign Function & Memory API uses an internal 
custom linked list to add native resources to a "memory session" 
abstraction (things that need to be cleaned up at a later point).


Linked list is quite critical in our use case because we need something 
that has a very fast insertion (in the head), and can scale gracefully 
to handle multiple threads.


In our case LinkedList is not good enough (because we want to deal with 
concurrent writes ourselves) - but aside from that, note that, at least 
looking at the numbers posted in your benchmarks, it seems that 
prepending an element to a classic LinkedList is 10x faster than 
ArrayList and 5x faster IndexList. Perhaps that's a case where IndexList 
has not been fully optimized - but for prepend-heavy code (and the javac 
compiler is another one of those), I think performance of addFirst is 
the number to look at.


As Tagir said, of course these use cases are very "niche" - and, at 
least in my experience, deevelopers in this "niche" tend to come up with 
ad-hoc specialized data structures anyways. So the return of investment 
for adding another collection type in this space seems relatively low.


Maurizio

On 09/07/2022 20:33, Tagir Valeev wrote:
Note that nobody these days cares about LinkedList. Use-cases where 
LinkedList outperforms careful use of ArrayList or ArrayDeque are next 
to none. So saying that your data structure is better than LinkedList 
is totally not a reason to add it to JDK. It should be better than 
ArrayList and ArrayDeque.


Having a single data structure that provides list and deque interface 
is a reasonable idea. However it would be much simpler to retrofit 
existing data structure like ArrayDeque, rather than create a new data 
structure. Here's an issue for this:

https://bugs.openjdk.org/browse/JDK-8143850

There were also discussions to enhance collections in general, adding 
more useful methods like getFirst() or removeLast() to ArrayList, etc. 
See for details:

https://bugs.openjdk.org/browse/JDK-8266572

To conclude, the idea of adding one more collection implementation 
looks questionable to me. It will add more confusion when people need 
to select which collection fits their needs better. It will require 
more learning. This could be justified if there are clear benefits in 
using it in real world problems, compared to existing collections. But 
so far I don't see the examples of such problems.


With best regards,
Tagir Valeev

сб, 9 июл. 2022 г., 11:22 Rodion Efremov :

Hello,

My benchmarking suggests, that, if nothing else, my
IndexedLinkedList outperforms gracefully the java.util.LinkedList,
so the use case should be the same (List + Deque
-interfaces) for both of the aforementioned data structures.

Best regards,
rodde


On Sat, Jul 9, 2022 at 11:19 AM Tagir Valeev 
wrote:

Hello!

Are there real world problems/use cases where
IndexedLinkedList would be preferred in terms of CPU/memory
usage over ArrayList?

сб, 9 июл. 2022 г., 07:18 Rodion Efremov :

Data structure repo:
https://github.com/coderodde/IndexedLinkedList

Benchmark repo:
https://github.com/coderodde/IndexedLinkedListBenchmark

I have profiled my data structure and it seems it’s more
performant than java.util.LinkedList or TreeList, if
nothing else.

So, is there any chance of including IndexedLinkedList to JDK?

Best regards,
rodde

[jdk19] RFR: 8290071: Javadoc for MemorySegment/MemoryAddress getter/setters contains some typos

[Maurizio Cimadamore]

Some of the accessors in `MemorySegment` and `MemoryAddress` (esp. setters) 
have typos - e.g. they use `from` instead of `into`.

I've tried to simplify the text and made it more regular.

-

Commit messages:
 - Initial push

Changes: https://git.openjdk.org/jdk19/pull/131/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk19=131=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290071
  Stats: 50 lines in 2 files changed: 0 ins; 0 del; 50 mod
  Patch: https://git.openjdk.org/jdk19/pull/131.diff
  Fetch: git fetch https://git.openjdk.org/jdk19 pull/131/head:pull/131

PR: https://git.openjdk.org/jdk19/pull/131

Re: RFR: 8289711: Add container configuration data to mbeans [v2]

[Alan Bateman]

On Wed, 6 Jul 2022 05:59:21 GMT, Alan Bateman  wrote:

>> xpbob has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   update header
>
> It's not clear that introducing this as a standard API is the right thing to 
> do. Are you 100% confident that the concepts of "CPU quota", "CPU shares", 
> "CPU period", "soft limit" etc. will last the test of time and that we don't 
> be back here next year with another PR to deprecate or replace this API?  I 
> don't disagree that exposing a MXBean could be useful for 
> monitoring/management purposes but I think we have to be cautious getting 
> getting locked in. A possible starting point for prototyping purposes and 
> getting feedback is a JDK-specific MXBean (module jdk.management).

> @AlanBateman @jerboaa That's a good idea.Adding a special bean is only 
> available on Linux systems.I do not know the process of creating a CSR, can 
> you help me create a CSR

It's too early to think about a CSR, probably a bit premature to create a PR 
too because it will take a few iterations to come to agreement on what API to 
expose, if any.

I don't think this should be a standard API, meaning 
java.management/java.lang.management.ContainerMXBean is not the right place for 
this. A JDK-specific MXBean is an option but it would only be usable on Linux 
and maybe only usable when running in a container environment. Working down, 
it's not clear to me how stable the attributes are and the mapping to both 
cgroup v1 and v2. There is also overlap with OperatingSystemMXBean which 
already defines the "TotalSwapSpaceSize" attribute.  There's another level of 
detail beyond that with unusual value -2 to distinguish "not available" from 
"not supported".  So I think there are a few things to think about there. 

Another direction to think about is not exposing an API that you can compile 
against but instead just register a MBean that provides access to the 
attributes that are interesting for the container environment. By this I mean 
ManagementFactory.getPlatformMBeanServer().registerMBean(...). This would be 
enough to browse the MBean and its attributes in any JMX console and may give 
you a bit more flexibility to expose attributes that are specific to the cgroup 
version. I'm not saying this is the right direction, I'm just listing here is 
something that you could be explored. If the main use case is JMX consoles 
rather than programmatic access then it may not be too bad.

-

PR: https://git.openjdk.org/jdk/pull/9372

Re: RFR: 8289711: Add container configuration data to mbeans [v2]

[Severin Gehwolf]

On Wed, 6 Jul 2022 05:59:21 GMT, Alan Bateman  wrote:

>> xpbob has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   update header
>
> It's not clear that introducing this as a standard API is the right thing to 
> do. Are you 100% confident that the concepts of "CPU quota", "CPU shares", 
> "CPU period", "soft limit" etc. will last the test of time and that we don't 
> be back here next year with another PR to deprecate or replace this API?  I 
> don't disagree that exposing a MXBean could be useful for 
> monitoring/management purposes but I think we have to be cautious getting 
> getting locked in. A possible starting point for prototyping purposes and 
> getting feedback is a JDK-specific MXBean (module jdk.management).

> @AlanBateman @jerboaa That's a good idea.Adding a special bean is only 
> available on Linux systems.I do not know the process of creating a CSR, can 
> you help me create a CSR

See https://wiki.openjdk.org/display/csr/CSR+FAQs

-

PR: https://git.openjdk.org/jdk/pull/9372

Re: RFR: 8289768: Clean up unused code [v3]

[Chris Hegarty]

On Fri, 8 Jul 2022 07:08:46 GMT, Daniel Jeliński  wrote:

>> This patch removes many unused variables and one unused label reported by 
>> the compilers when relevant warnings are enabled. 
>> 
>> The unused code was found by compiling after removing `unused` from the list 
>> of disabled warnings for 
>> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190)
>>  and 
>> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203),
>>  and enabling 
>> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170)
>>  MSVC warning.
>> 
>> I only removed variables that were uninitialized or initialized without side 
>> effects. I verified that the removed variables were not used in any 
>> `#ifdef`'d code. I checked that the changed code still compiles on Windows, 
>> Linux and Mac, both in release and debug versions.
>
> Daniel Jeliński has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Update copyright
>  - Remove more unused variables

LGTM

-

Marked as reviewed by chegar (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9383

Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[] [v2]

[Сергей Цыпанов]

On Fri, 8 Jul 2022 14:04:14 GMT, Roger Riggs  wrote:

>> But if I roll back the added constructor I'll go through existing public one 
>> `public String(byte[] bytes, int offset, int length, Charset charset)` doing 
>> bounds check twice, won't I?
>
> The new constructor looks very odd, especially when it does not have an 
> explanation and doesn't describe the required preconditions for calling it.  
> Is there a better way than adding a non-functional argument?
> The "unused" name is going to draw a warning from IntelliJ and some 
> enterprising developer is going to try to remove it, not understanding why 
> its there. And there is a bit of overhead pushing the extra arg.
> 
> The constructor should be private, it has a very narrow scope of use given 
> the lack of checking its args.
> 
> It would be nice if the Hotspot compiler would recognize the llmits on the 
> range and optimize away the checks;
> it would have broader applicability then this point fix.
> I would be interesting to ask the compiler folks if that's a future 
> optimization.
> These source changes make it harder to understand what's happening where; 
> though that is sometimes work it for a noticeable performance improvement.

Well, we already have e.g. `String(char[], int, int, Void)` and 
`String(AbstractStringBuilder asb, Void sig)` where trailing argument is for 
disambiguation against private constructors. I did the same for mine and 
applied the same naming as in other trailing `Void` params.

-

PR: https://git.openjdk.org/jdk/pull/9407

Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[] [v2]

[Сергей Цыпанов]

> We can skip bounds check and null check for Charset in case we use the array 
> entirely and the Charset is either default one or proven to be non-null.
> 
> Benchmark results:
> 
> before
> 
> Benchmark  Mode  Cnt  Score   
> Error  Units
> StringConstructor.newStringFromArray   avgt   50  4,815 ± 
> 0,154  ns/op
> StringConstructor.newStringFromArrayWithCharsetavgt   50  4,462 ± 
> 0,068  ns/op
> StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,653 ± 
> 0,040  ns/op
> StringConstructor.newStringFromRangedArray avgt   50  5,090 ± 
> 0,066  ns/op
> StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,550 ± 
> 0,041  ns/op
> StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  8,080 ± 
> 0,055  ns/op
> 
> after
> 
> Benchmark  Mode  Cnt  Score   
> Error  Units
> StringConstructor.newStringFromArray   avgt   50  4,595 ± 
> 0,053  ns/op
> StringConstructor.newStringFromArrayWithCharsetavgt   50  4,038 ± 
> 0,062  ns/op
> StringConstructor.newStringFromArrayWithCharsetNameavgt   50  8,035 ± 
> 0,031  ns/op
> StringConstructor.newStringFromRangedArray avgt   50  4,084 ± 
> 0,007  ns/op
> StringConstructor.newStringFromRangedArrayWithCharset  avgt   50  4,014 ± 
> 0,008  ns/op
> StringConstructor.newStringFromRangedArrayWithCharsetName  avgt   50  7,466 ± 
> 0,071  ns/op

Сергей Цыпанов 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 three additional 
commits since the last revision:

 - Merge branch 'master' into 8289908
 - 8289908: Make constructor private and use trailing Void instead of int
 - 8289908: Skip bounds check for cases when String is constructed from 
entirely used byte[]

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9407/files
  - new: https://git.openjdk.org/jdk/pull/9407/files/567727e9..294e91f7

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9407=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=9407=00-01

  Stats: 11699 lines in 1295 files changed: 3911 ins; 1617 del; 6171 mod
  Patch: https://git.openjdk.org/jdk/pull/9407.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9407/head:pull/9407

PR: https://git.openjdk.org/jdk/pull/9407

Re: RFR: 8289768: Clean up unused code [v3]

[Michael McMahon]

On Fri, 8 Jul 2022 07:08:46 GMT, Daniel Jeliński  wrote:

>> This patch removes many unused variables and one unused label reported by 
>> the compilers when relevant warnings are enabled. 
>> 
>> The unused code was found by compiling after removing `unused` from the list 
>> of disabled warnings for 
>> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190)
>>  and 
>> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203),
>>  and enabling 
>> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170)
>>  MSVC warning.
>> 
>> I only removed variables that were uninitialized or initialized without side 
>> effects. I verified that the removed variables were not used in any 
>> `#ifdef`'d code. I checked that the changed code still compiles on Windows, 
>> Linux and Mac, both in release and debug versions.
>
> Daniel Jeliński has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Update copyright
>  - Remove more unused variables

libnet changes look fine to me

-

Marked as reviewed by michaelm (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9383