<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Erik,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Thanks for the quick response.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small"><span style="font-family:Arial,Helvetica,sans-serif">On Wed, Jun 14, 2023 at 10:50 AM Erik Gahlin <<a href="mailto:erik.gahlin@oracle.com">erik.gahlin@oracle.com</a>> wrote:</span><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg4212818953006157534">




<div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
The intent of the code is to remove the sample from both the time-based list and the priority queue as the object is no longer alive. <span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">The
 weight of the object, the allocation span, in the time-based list, should be divided to its neighbour(s). </span><span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">The
 rationale for this is to keep samples evenly distributed over time, or to be exact, the amount allocation has happened. It will be as if the object was never sampled (as if the TLAB size would be larger)</span></div></div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">Hmmmm, I'm still not sure I understand the rationale. What is the problem that you were trying to solve with this logic?</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg4212818953006157534"><div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">The priority queue exists to find the sampled object with the lowest allocation span quickly when the list fills
 up. It needs to be replaced with a sampled object with higher weight/span.</span></div></div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">I have no doubts about the priority queue. The code and logic makes sense, though one could argue if the complexity of the priority queue really pays off. If you have leaks, not sure it really matters if you have few big leaks or you have many small ones. E.g. the async profiler's object sampler does not keep the queue ordered: </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">```</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">        if (_lock.tryLock()) {<br>            u32 start = (((uintptr_t)object >> 4) * 31 + ((uintptr_t)jni >> 4) + trace) & (MAX_REFS - 1);<br>            u32 i = start;<br>            do {<br>                jweak w = _refs[i];<br>                if (w == NULL || collected(w)) {<br>                    if (w != NULL) jni->DeleteWeakGlobalRef(w);<br>                    _refs[i] = wobject;<br>                    _values[i].size = size;<br>                    _values[i].trace = trace;<br>                    _values[i].time = TSC::ticks();<br>                    _lock.unlock();<br>                    return;<br>                }<br>            } while ((i = (i + 1) & (MAX_REFS - 1)) != start);<br><br>            _full = true;<br>            _lock.unlock();<br>        }<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">```</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">This would be better discussed separately. I can start a separate thread if people want to revisit this.</div></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small"><span style="font-family:Arial,Helvetica,sans-serif"> </span><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg4212818953006157534"><div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">This may, or may not, be what the code does.
</span><span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">It was written many years ago, and I need to re-read the code to check if that is really the case, but before
 digging deeper, I want to check if we are on the same page when it comes to the intent</span></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><br>
</span></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">Erik</span></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)">
<br>
</div>
<div id="m_4212818953006157534appendonsend"></div>
<hr style="display:inline-block;width:98%">
<div id="m_4212818953006157534divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> hotspot-jfr-dev <<a href="mailto:hotspot-jfr-dev-retn@openjdk.org" target="_blank">hotspot-jfr-dev-retn@openjdk.org</a>> on behalf of Galder Zamarreno <<a href="mailto:galder@redhat.com" target="_blank">galder@redhat.com</a>><br>
<b>Sent:</b> Wednesday, June 14, 2023 6:30 AM<br>
<b>To:</b> <a href="mailto:hotspot-jfr-dev@openjdk.org" target="_blank">hotspot-jfr-dev@openjdk.org</a> <<a href="mailto:hotspot-jfr-dev@openjdk.org" target="_blank">hotspot-jfr-dev@openjdk.org</a>><br>
<b>Subject:</b> Priority redistribution in Old Object Sample event</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
Hi,</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
I've been porting over JFR Old Object Sample event to GraalVM and I had the following doubt about the code in HotSpot:</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
I'm trying to understand the code in [1]:</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
```</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
  // push span onto previous<br>
  if (previous != NULL) {<br>
    _priority_queue->remove(previous);<br>
    previous->add_span(sample->span());<br>
    _priority_queue->push(previous);<br>
  }<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
```</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
This code is related to old object sampling. When the queue is full, samples that have been GC'd are removed from the queue. To do so it takes its previous (from the list view) and adds the span of the removed dead object onto it. Basically, I'm wondering why
 this is done this way.</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
My initial gut feeling when I first saw the code is that it does it so that the priority queue does not get too imbalanced (it uses a heap underneath) when removing objects, but that only makes sense to me if the previous is not the previous object sampled
 based on time, but the previous object sampled based on the order of the priority queue, so that's not what seems to happen.<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
I'm wondering now if the fact that the list view is ordered by insertion time is relevant here. Is it maybe trying to give more importance (by increasing its span) to surviving objects that are near in time to those that have been garbage collected?<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
Something that's probably related to this is how the span is initially calculated. This is done in [2], which you can find below:</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
```</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
_total_allocated += allocated;<br>
const size_t span = _total_allocated - _priority_queue->total();<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
```</div>
<div>
<div dir="ltr">
<div dir="ltr">
<div>
<div dir="ltr">
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
Again when I first looked at it, I thought: well that's just equivalent to:</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
```</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
const size_t span = allocated;<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
```</div>
<br>
</div>
<div dir="ltr">
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
But clearly things are more nuanced than that.</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
The only documentation I've found so far is in Marcus' blog post [3]:</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
> If sampled objects are garbage collected, they are removed and the priority redistributed to the neighbours. The priority is called span, and is currently the size of the allocation, giving more weight to larger (therefore more severe) leaks.</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
Unfortunately what is a neighbour is not explained, nor why the priority gets redistributed.</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
<br>
</div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
Thanks</div>
</div>
<div dir="ltr">Galder</div>
</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
[1] <a href="https://github.com/openjdk/jdk17u-dev/blob/master/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp#L264-L269" target="_blank">
https://github.com/openjdk/jdk17u-dev/blob/master/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp#L264-L269</a></div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
[2] <a href="https://github.com/openjdk/jdk17u-dev/blob/ef86ea2842b1a204834291d9d6665bfcd7b75fbc/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp#L212-L213" target="_blank">https://github.com/openjdk/jdk17u-dev/blob/ef86ea2842b1a204834291d9d6665bfcd7b75fbc/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp#L212-L213</a></div>
<div style="font-family:arial,helvetica,sans-serif;font-size:small">
[3] <a href="https://hirt.se/blog/?p=1055" target="_blank">https://hirt.se/blog/?p=1055</a></div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>

</div></blockquote></div></div>