<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hello Frank,</p>
<p>I see that a JBS issue has been created for this same issue
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.org/browse/JDK-8325754">https://bugs.openjdk.org/browse/JDK-8325754</a>. <br>
</p>
<p>I don't have enough knowledge of this area and haven't reviewed
this part of the code in detail to see if there are any obvious
issues with what you are proposing as a solution. Since there's
now a JBS issue created for this and you seem to have done enough
investigation and work on this one already, would you be
interested in creating a pull request against the
<a class="moz-txt-link-freetext" href="https://github.com/openjdk/jdk">https://github.com/openjdk/jdk</a> repo with this proposed change?
(you'll have to sign a OCA). This guide <a class="moz-txt-link-freetext" href="https://openjdk.org/guide/">https://openjdk.org/guide/</a>
should help you get started. It can then go through the usual
reviews that a bug fix/enhancement goes through.<br>
</p>
<p>-Jaikiran<br>
</p>
<div class="moz-cite-prefix">On 11/02/24 7:27 pm, Frank Kretschmer
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:6d3b62cb-15f4-471c-97b6-b72977223f91@gmx.net">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<p>Hello Core-Libs-Dev team,<br>
<br>
may I ask you about your opinion about a tiny one-liner change
in AbstractQueuedSynchronizer, just as a suggestion how to make
ConditionObjects / Nodes even more garbage collector friendly?<br>
<br>
Checked out
<a class="moz-txt-link-freetext"
href="https://github.com/openjdk/jdk/blob/jdk-17%2B35/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java"
moz-do-not-send="true">https://github.com/openjdk/jdk/blob/jdk-17%2B35/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java</a>
(the same on master branch with different line numbers near to
line 1506):<br>
<br>
<font size="2" face="Courier New, Courier, monospace">@@
-1431,40 +1431,41 @@ public abstract class
AbstractQueuedSynchronizer<br>
public class ConditionObject implements Condition,
java.io.Serializable {<br>
// ...<br>
private void doSignal(ConditionNode first, boolean
all) {<br>
while (first != null) {<br>
ConditionNode next = first.nextWaiter;<br>
+ first.nextWaiter = null; // GC-friendly:
avoid chains of dead ConditionNodes<br>
if ((firstWaiter = next) == null)<br>
lastWaiter = null;<br>
if ((first.getAndUnsetStatus(COND) &
COND) != 0) {<br>
enqueue(first);<br>
// ...</font><br>
<br>
By setting the nextWaiter to null of the first condition node,
which is transferred from the condition queue to the sync queue
in this method, long chains of ConditionNode instances can be
avoided. Though a single ConditionNode is small, these chains of
ConditionNodes become very huge on the heap (I've seen more than
1GB on an application server over time) if at least one node was
promoted to the old generation for any reason. They survive
minor collections and are cleaned up only on mixed / full
collections, and thus put unnecessary pressure on G1 garbage
collector.<br>
<br>
The same change could also be applied to
'AbstractQueuedLongSynchronizer'.<br>
<br>
I know premature optimization is the root of all evil, on the
other hand I could image that many applications benefit from
GC-friendly ConditionObjects, since they are frequently used in
various classes like PriorityBlockingQueue / LinkedBlockingDeque
/ LinkedBlockingQueue, the latter one as default work queue for
executor services like fixed thread pools for processing
asynchronous tasks.<br>
<br>
Thank you all for your time and help!<br>
<br>
Best regards<br>
Frank<br>
</p>
<div class="moz-cite-prefix">Am 08.02.2024 um 12:15 schrieb Frank
Kretschmer:<br>
</div>
<blockquote type="cite"
cite="mid:64bc8cbd-78f4-44f4-90ae-80466c563281@gmx.net">Hello
Thomas, hello Core-Libs-Dev, <br>
<br>
thank you for cc'ing my email. In deed my idea/suggestion is to
modify <br>
the AbstractQueuedSynchronizer$ConditionNode handling in such a
way that <br>
it gets unlinked from the chain of condition nodes if it is not
needed <br>
any more (it might be the "nextWaiter" node), in order to be
more <br>
GC-friendly. <br>
<br>
@core-libs-dev: I've just attached the
“G1LoiteringConditionNodes” demo <br>
class and "gc.log" again so that you can have a look if you
like. <br>
<br>
Best regards <br>
<br>
Frank <br>
<br>
<br>
Am 08.02.2024 um 11:04 schrieb Thomas Schatzl: <br>
<blockquote type="cite">Hi, <br>
<br>
since this looks like a suggestion for a change to the
libraries <br>
similar to the mentioned JDK-6805775, and not actually GC,
cc'ing the <br>
core-libs-dev mailing list. <br>
<br>
Hth, <br>
Thomas <br>
<br>
On 07.02.24 15:20, Frank Kretschmer wrote: <br>
<blockquote type="cite">Hi Java GC-experts, <br>
<br>
I'm facing an interesting G1 garbage collector observation
in OpenJDK <br>
17.0.9+9, which I would like to share with you. <br>
<br>
My application runs many asynchronous tasks in a fixed
thread pool, <br>
utilizing its standard LinkedBlockingQueue. Usually, it
generates just a <br>
little garbage, but from time to time, I observed that the
survivor <br>
spaces grow unexpectedly, and minor collection times
increase. <br>
<br>
This being the case, many <br>
java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionNode <br>
instances can be found on the heap. In fact, the whole heap
(rank 1 as <br>
shown in jmap) was filled up with ConditionNode instances
after a while. <br>
<br>
After some tests, I figured out that G1 seems to be able to
collect <br>
“dead” ConditionNode instances during minor collections only
if no <br>
formerly alive ConditionNode instances were promoted to the
old <br>
generation and died there. <br>
<br>
To illustrate that, I've attached a
“G1LoiteringConditionNodes” class <br>
that can be run for demo purposes, e.g. under Linux with
OpenJDK <br>
17.0.9+9 (VM options see comments within the class), and its
gc-log <br>
output. It shows that during the first two minutes,
everything is fine, <br>
but after a promotion to the old generation, survivors grow
and minor <br>
pause time increase from 3 to 10ms. <br>
<br>
For me, it looks like an issue similar to <br>
<a class="moz-txt-link-freetext"
href="https://bugs.openjdk.org/browse/JDK-6805775"
moz-do-not-send="true">https://bugs.openjdk.org/browse/JDK-6805775</a>
“LinkedBlockingQueue Nodes <br>
should unlink themselves before becoming garbage”, which was
fixed in <br>
OpenJDK 7. <br>
<br>
What’s your opinion about that? Wouldn’t it be worth to
enable G1 to <br>
collect those AbstractQueuedSynchronizer$ConditionNode
instances during <br>
minor collections, as it is done for LinkedBlockingQueue
Nodes? <br>
<br>
Best regards <br>
<br>
Frank<br>
</blockquote>
<br>
</blockquote>
</blockquote>
</blockquote>
</body>
</html>