RFR: 8257149: Improve G1 Service thread task scheduling to guarantee task delay

[Stefan Johansson]

On Thu, 26 Nov 2020 11:33:44 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Adopt the "at-least" semantics for scheduling delays in G1 service thread scheduling API so that a task is never scheduled earlier than it asked for. IOW, `schedule_task(task, delay_ms)` means `task` will run after `>= delay_ms` has passed.
> 
> Inspecting the logs (`-Xlog:gc,gc+task*=trace`) and focusing lines containing `(Remembered Set Sampling Task) (schedule)`, we can see at what time the sampling task is scheduled.
>  
> Without `ceil`, sampling task could be scheduled more frequent than intended:
> [0.321s][trace][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (schedule) @0.322s
> [0.321s][debug][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (run) 0.042ms (cpu: 0.000ms)
> [0.321s][debug][gc,task,start] G1 Service Thread (Remembered Set Sampling Task) (run)
> [0.321s][trace][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (schedule) @0.322s
> [0.321s][debug][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (run) 0.010ms (cpu: 0.000ms)
> [0.321s][debug][gc,task,start] G1 Service Thread (Remembered Set Sampling Task) (run)
> [0.321s][trace][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (schedule) @0.322s
> [0.321s][debug][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (run) 0.008ms (cpu: 0.000ms)
> [0.321s][debug][gc,task,start] G1 Service Thread (Remembered Set Sampling Task) (run)
> 
> With ceil; there's 300ms (`G1ConcRefinementServiceIntervalMillis`) interval btw each scheduling of the sampling task.
> [0.093s][trace][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (schedule) @0.323s
> [0.093s][debug][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (run) 0.006ms (cpu: 0.000ms)
> [0.093s][debug][gc,task,start] G1 Service Thread (Periodic GC Task) (run)
> [0.093s][trace][gc,task      ] G1 Service Thread (Periodic GC Task) (schedule) @1.093s
> [0.093s][debug][gc,task      ] G1 Service Thread (Periodic GC Task) (run) 0.004ms (cpu: 0.000ms)
> [0.093s][trace][gc,task      ] G1 Service Thread (wait) 0.230s
> [0.323s][debug][gc,task,start] G1 Service Thread (Remembered Set Sampling Task) (run)
> [0.323s][trace][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (schedule) @0.623s
> [0.323s][debug][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (run) 0.024ms (cpu: 0.000ms)
> [0.323s][trace][gc,task      ] G1 Service Thread (wait) 0.300s
> [0.623s][debug][gc,task,start] G1 Service Thread (Remembered Set Sampling Task) (run)
> [0.623s][trace][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (schedule) @0.923s
> [0.623s][debug][gc,task      ] G1 Service Thread (Remembered Set Sampling Task) (run) 0.026ms (cpu: 0.000ms)
> [0.623s][trace][gc,task      ] G1 Service Thread (wait) 0.300s
> 
> Tested: tier1 and manually checking the logs.

Thanks for fixing this Albert, just a few small comments. If you don't agree just leave them as is.

src/hotspot/share/gc/g1/g1RemSet.cpp line 583:

> 581: 
> 582:     // Reschedule if a GC happened too recently.
> 583:     auto delay_ms = reschedule_delay_ms();

I would prefer to be explicit about `jlong` since sometimes time comes in the form of `doubles`.

src/hotspot/share/gc/g1/g1ServiceThread.hpp line 139:

> 137: 
> 138:   // Register a task with the service thread. The task is guaranteed not to run
> 139:   // until >= `delay_ms` has passed. If no delay is specified or the delay is

I would prefer to use "at least" instead of ">=".

src/hotspot/share/gc/g1/g1ServiceThread.hpp line 144:

> 142: 
> 143:   // Schedule an already-registered task to run in >= `delay_ms` time.
> 144:   void schedule_task(G1ServiceTask* task, jlong delay_ms);

I would like this comment to still include that the thread will be notified.

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

Marked as reviewed by sjohanss (Reviewer).

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