RFR: 8254758: Change G1ServiceThread to be task based [v2]
Albert Mingkun Yang
ayang at openjdk.java.net
Tue Oct 20 06:49:17 UTC 2020
On Tue, 20 Oct 2020 06:45:29 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:
>> Please review this enhancement to make `G1ServiceThread` task based. This thread has evolved from having a single
>> responsibility to do more things. And there are plans to add more to it. To make this easier and to allow different
>> tasks to run at different intervals this change adds a simple task mechanism to the service thread. The idea is that
>> the service thread keeps a list of `G1ServiceTask`-objects that are ordered by when they should execute. When no tasks
>> are ready to be run the service thread check how far into the future the next task is scheduled and sleeps/waits for
>> that duration. If a new task is added while the thread is waiting it will be woken up to check for ready tasks. There
>> are currently two tasks in the list. They have been extracted from the old service thread into the tasks:
>> `G1PeriodicGCTask` and `G1RemSetSamplingTask`. These are more or less identical to the old code, but in the form of
>> tasks. The only difference is that `G1PeriodicGCTask` no longer need to keep track of the last attempt, since it will
>> be rescheduled with the correct interval. To reschedule a task is to update the time when it should be executed next
>> and adding it back to the list. This insertion is O(n) and it should be fine since the expected number of tasks handled
>> by the service thread is low. If a task don't want to be rescheduled it can set a negative timeout and that will cause
>> the task to be dropped from the list. There are currently no such tasks and adding such tasks come with the
>> responsibility of making sure resources are taken care of when the task is no longer active. I've added a gtest to do
>> some basic VM-testing and apart from a lot of local testing I've run mach5 tier 1-4 a few times with good results. I
>> have created a few follow up tasks to look at moving the current tasks to more appropriate locations and also updated a
>> few old service thread related tasks with the label:
>> [`gc-g1-service-thread`](https://bugs.openjdk.java.net/issues/?jql=labels%20%3D%20gc-g1-service-thread)
>
> Stefan Johansson has updated the pull request incrementally with one additional commit since the last revision:
>
> Albert review
Thank you for the update regarding 1 and 2. As for `timeout=0`, it's not obvious to me how it will be resolved if there
are multiple `timeout=0` tasks. Since there are no such tasks yet, I am fine with the current status. Ideally, please
provide some doc on the behavior of `timeout=0` next to `virtual int64_t timeout_ms() = 0;`; sth like what's said for
"negative" values.
-------------
Marked as reviewed by ayang (Author).
PR: https://git.openjdk.java.net/jdk/pull/734
More information about the hotspot-gc-dev
mailing list