RFR: 8254758: Change G1ServiceThread to be task based

Stefan Johansson sjohanss at openjdk.java.net
Tue Oct 20 06:45:29 UTC 2020


On Mon, 19 Oct 2020 20:18:12 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> 1. There's a sentinel node in the task list. I wonder if it's possible to get rid of it. This way, `G1ServiceTask` can
>> be made abstract, and `G1ServiceTask::execute` and `G1ServiceTask::timeout` can be pure and unreachable statically. 2.
>> Currently, the task timeout is a `double`, which says very little on what precision is actually supported. I think an
>> integral type would be better. 3. "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." I wonder if `timeout=0` is allowed, and what that means.
>
>>     1. There's a sentinel node in the task list. I wonder if it's possible to get rid of it. This way, `G1ServiceTask` can
>>     be made abstract, and `G1ServiceTask::execute` and `G1ServiceTask::timeout` can be pure and unreachable statically.
>> 
> I tried this out first, but I like not having to check for `NULL` when adding. Another idea I had to was create a
> specific `G1SentinelTask`, to allow `execute()` and `timeout()` to be pure virtual. If you like this approach I'm happy
> to go in that direction.
>>     2. Currently, the task timeout is a `double`, which says very little on what precision is actually supported. I think
>>     an integral type would be better.
>> 
> I agree, I would prefer to avoid double all together but it is a quite nice to use `os::elapsedTime()` for the timings
> since the logging-framework prints time this way. And since you then have a double as the base value having the timeout
> using the same is quite nice, but I'll take a look an see if I can make it look nice with integral type. But the
> scheduled time will still be using a double.
>>     3. "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." I wonder if `timeout=0` is allowed, and what that means.
> 
> Timeout 0 basically means run me again instantly unless someone else is up. For example looking forward the uncommit
> task will reschedule itself with a 0 timeout avoiding putting the service thread to sleep but still making it possible
> for another task to run between two invocations.

Updated after Alberts input:
- Sentinel task is now a separate class, allowing `G1ServiceTask` to be pure virtual.
- Changed `double G1ServiceTask::timeout()` to `int64_t G1ServiceTask::timeout_ms()`

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

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



More information about the hotspot-gc-dev mailing list