RFR: 8344009: Improve compiler memory statistics [v4]

[Ashutosh Mehra]

On Tue, 25 Feb 2025 16:13:48 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/compiler/compilationMemStatInternals.hpp line 160:
>> 
>>> 158:     void init(T v)        { start = cur = peak = v; }
>>> 159:     void update(T v)      { cur = v; if (v > peak) peak = v; }
>>> 160:     dT end_delta() const  { return (dT)cur - (dT)start; }
>> 
>> Should it be `return (dT)(cur - start); }`
>
> hmm, I like to avoid the inner overflow is cur < start (if the phase released more memory than it allocated)

right, I missed that.

>> src/hotspot/share/compiler/compilationMemStatInternals.hpp line 161:
>> 
>>> 159:     void update(T v)      { cur = v; if (v > peak) peak = v; }
>>> 160:     dT end_delta() const  { return (dT)cur - (dT)start; }
>>> 161:     size_t temporary_peak_size() const { return MIN2(peak - cur, peak - start); }
>> 
>> shouldn't it be `MAX2(peak - cur, peak - start)`? Why not just `peak - start`?
>
> We are only interested in a rise that rose significantly above **both** the start and end point of the measurements. 
> 
> E.g.:
> - if we have this: start = 0, end = 20MB, peak = 20MB, this is not a temporary peak and we already know that the end usage is 20MB.
> - if we have this: start = 20MB, end = 0, peak = 20MB, this is not a temporary peak either, because we already know the starting footprint was 20MB.
> - but if we have start = 0, end = 0, peak = 20MB, this is interesting since if we just print start and end we will miss the fact that in between those times we had temporarily allocated 20MB.

Thanks for the explanation. It would be great if some comment can be added, possibly along with some example like the one in the previous comment, to explain the meaning of `temporary_peak_size` and the corresponding calculation.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23530#discussion_r1970880396
PR Review Comment: https://git.openjdk.org/jdk/pull/23530#discussion_r1970880439