RFR: 8280139: Report more detailed statistics about task stealing in task queue stats

[Kim Barrett]

On Wed, 19 Jan 2022 12:55:32 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews that improves task queue statistics?
> 
> This is a step to see problems with task queue in general and measure future task stealing improvements in GC; particularly contention on steal information has shown to be correlated to performance, so information about basic occurrences during stealing (how many empty, how many contended, how many contended in a row, max contended accesses in a row) in the task queue is very interesting for further performance work.
> 
> Old output of `gc+tasks+stats=trace` after compiling in these statistics:
> 
> thr      qpush       qpop     qpop-s   qattempt     qsteal      opush       omax
> --- ---------- ---------- ---------- ---------- ---------- ---------- ----------
>   0    3777115    3776816       3872      17434       9630          0          0
> 
> And new:
> 
> thr        push         pop    pop-slow  st-attempt    st-empty     st-ctdd  st-success st-ctdd-max st-biasdrop  ovflw-push   ovflw-max
> --- ----------- ----------- ----------- ----------- ----------- ----------- ----------- ----------- ----------- ----------- -----------
>   0     4616264     4616218        3608       12945           1        1650       11294           5        9405           0           0
> [...]
> 
> The first four columns stayed the same, then the `qsteal` has been split up into the various steal results (empty/contended/success). `st_ctdd_max` gives the maximum amount of contended steal attempts in a row. `st_biasdrop` shows how many times the steal bias has been dropped (reset).
> `ovflw-push` and `ovfl-max` are the same as before, just renamed a bit to use the available space.
> 
> There has been no intention to change anything about the algorithm, so there is no need for perf testing (I can see). Later changes will do that.
> 
> Thanks,
>   Thomas

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/shared/taskqueue.hpp line 76:

> 74:   inline void record_pop_slow()      { record_pop(); ++_stats[pop_slow]; }
> 75:   inline void record_steal_attempt(uint kind) { ++_stats[steal_attempt]; ++_stats[steal_empty + kind]; }
> 76:   inline void record_contended_in_a_row(uint in_a_row) { if (_stats[steal_max_contended_in_a_row] < in_a_row) _stats[steal_max_contended_in_a_row] = in_a_row; }

Please add some line breaks to this very long definition.

src/hotspot/share/gc/shared/taskqueue.hpp line 366:

> 364:   // recently pushed).
> 365:   // The result value order of this correspond to the order in the corresponding StatId.
> 366:   enum PopResult {

I think the `PopResult` enum should be in `TaskQueueSuper`.  I think that will allow some of the references to be simplified.  Others might be simplified by being brought into scope via `using`.

src/hotspot/share/gc/shared/taskqueue.inline.hpp line 301:

> 299:       sel_k = k2;
> 300:       suc = _queues[k2]->pop_global(t);
> 301:       TASKQUEUE_STATS_ONLY(queue(queue_num)->stats.record_steal_attempt((uint)suc);)

Consider adding `TaskQueueSuper::record_steal_attempt(PopResult)` to package up this idiom.

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

PR: https://git.openjdk.java.net/jdk/pull/7143