<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Viktor,</p>
<p><br>
</p>
<p>may I add one option to your evaluation?</p>
<p><font size="1"><br>
</font></p>
<blockquote>
<p><font size="1" face="Courier New, Courier, monospace">@@
-1506,14 +1506,15 @@ public ConditionObject() { }<br>
private void doSignal(ConditionNode first, boolean
all) {<br>
while (first != null) {<br>
ConditionNode next = first.nextWaiter;<br>
if ((firstWaiter = next) == null)<br>
lastWaiter = null;<br>
if ((first.getAndUnsetStatus(COND) &
COND) != 0) {<br>
enqueue(first);<br>
<font color="#ff0000">+ first.nextWaiter =
null; // GC-friendly</font><br>
if (!all)<br>
break;<br>
}<br>
first = next;<br>
}<br>
}</font><br>
</p>
</blockquote>
<p><br>
</p>
<p>(<font size="2" face="Courier New, Courier, monospace">AbstractQueuedSynchronizer
</font>line numbers as in gitlab current master)</p>
<p><br>
</p>
<p>This variant takes care about race conditions on cancellation (unlinkCancelledWaiters()
needs '<font size="1" face="Courier New, Courier, monospace">nextWaiter</font>'),
as thanks to "<font size="1"
face="Courier New, Courier, monospace">getAndUnsetStatus(COND)
& COND) != 0</font>" only alternatively/once executed.<br>
</p>
<p><br>
</p>
<p>So this option is definitively better / more robust than my first
one <span id="🙂">🙂</span></p>
<p><br>
</p>
<p><br>
</p>
<p>Best regards</p>
<p>Frank<br>
</p>
<p><font size="2"><br>
</font></p>
<p><br>
</p>
<p><br>
</p>
<div class="moz-cite-prefix">Am 19.02.2024 um 12:41 schrieb Viktor
Klang:<br>
</div>
<blockquote type="cite"
cite="mid:MW4PR10MB57741938DB87F5145BE1B641FF512@MW4PR10MB5774.namprd10.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
<div class="elementToProof"
style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Frank,<br>
<br>
We'll see what the option are. <span id="🙂">🙂</span><br>
</div>
<div class="elementToProof"
style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="Signature">
<div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Cheers,<br>
√</div>
<div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<b><br>
</b></div>
<div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<b>Viktor Klang</b></div>
<div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Software Architect, Java Platform Group<br>
Oracle</div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt"
face="Calibri, sans-serif" color="#000000"><b>From:</b> Frank
Kretschmer <a class="moz-txt-link-rfc2396E" href="mailto:frank.kretschmer@gmx.net"><frank.kretschmer@gmx.net></a><br>
<b>Sent:</b> Sunday, 18 February 2024 15:36<br>
<b>To:</b> Jaikiran Pai <a class="moz-txt-link-rfc2396E" href="mailto:jai.forums2013@gmail.com"><jai.forums2013@gmail.com></a>;
Viktor Klang <a class="moz-txt-link-rfc2396E" href="mailto:viktor.klang@oracle.com"><viktor.klang@oracle.com></a>; Java Core Libs
<a class="moz-txt-link-rfc2396E" href="mailto:core-libs-dev@openjdk.java.net"><core-libs-dev@openjdk.java.net></a><br>
<b>Subject:</b> [External] : Re: OpenJDK 17: Loitering
AbstractQueuedSynchronizer$ConditionNode instances (also on
latest master branch) [JDK-8325754]</font>
<div> </div>
</div>
<div>
<p>Hello Jaikiran, hello Viktor,<br>
<br>
in the meantime, I've seen that the JBS issue has been
assigned to Viktor Klang. @Viktor: I totally agree with your
comment that the proposed solution may not be the best
possible option, and that further explorations were required.<br>
<br>
My intention to propose unlinking ConditionNodes by null'ing
their ‘nextWaiter’ reference was just to verify that the chain
of ‘nextWaiter’ references leads to the observed garbage
collection behavior, and that the GC is able to collect the
nodes during minor / young collections if the references are
cleaned in time.<br>
<br>
I checked also a few other variants (null'ing the ‘nextWaiter’
reference at the end of all await...() methods in
ConditionObject, or in/just before enqueue()), but at the end
of the day, I felt that null'ing it in doSignal() explains
what I want to show the easiest. On the other hand, the other
options could be better in order to avoid race conditions with
canceled nodes.<br>
<br>
For sure there are many other options that I am not aware of,
so please take my proposal just as an example.<br>
<br>
Looking forward to your explorations.</p>
<p>Best regards</p>
<p>Frank</p>
<p><br>
</p>
<div class="x_moz-cite-prefix">Am 14.02.2024 um 07:43 schrieb
Jaikiran Pai:<br>
</div>
<blockquote type="cite">
<p>Hello Frank,</p>
<p>I see that a JBS issue has been created for this same issue
<a class="x_moz-txt-link-freetext moz-txt-link-freetext"
href="https://bugs.openjdk.org/browse/JDK-8325754"
moz-do-not-send="true">
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="x_moz-txt-link-freetext"
href="https://urldefense.com/v3/__https://github.com/openjdk/jdk__;!!ACWV5N9M2RV99hQ!NtWokgpYjFyT0Gdq0NiTif6NvtYcNz39rzE7qzmJsQi5X_KwWSMhmV16WfkPx_5ByfNe4J-pgT8gyqCLKofbXZ9rkczUDg$"
moz-do-not-send="true">
https://github.com/openjdk/jdk</a> repo with this proposed
change? (you'll have to sign a OCA). This guide
<a class="x_moz-txt-link-freetext moz-txt-link-freetext"
href="https://openjdk.org/guide/" moz-do-not-send="true">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="x_moz-cite-prefix">On 11/02/24 7:27 pm, Frank
Kretschmer wrote:<br>
</div>
<blockquote type="cite">
<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="x_moz-txt-link-freetext"
href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/jdk-17*2B35/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java__;JQ!!ACWV5N9M2RV99hQ!NtWokgpYjFyT0Gdq0NiTif6NvtYcNz39rzE7qzmJsQi5X_KwWSMhmV16WfkPx_5ByfNe4J-pgT8gyqCLKofbXZ8EWBUt0w$"
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="x_moz-cite-prefix">Am 08.02.2024 um 12:15
schrieb Frank Kretschmer:<br>
</div>
<blockquote type="cite">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="x_moz-txt-link-freetext 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>
</blockquote>
</div>
</blockquote>
</body>
</html>