JDK-8074314 : PeriodicTask and WatcherThread APIs need to be cleaned up
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • OS: generic
  • CPU: generic
  • Submitted: 2015-03-03
  • Updated: 2021-11-09
  • Resolved: 2020-02-11
Related Reports
Relates :  
Relates :  
Relates :  
Description
During the code review cycle for the following bug fix:

    JDK-8072439 fix for 8047720 may need more work

it was mentioned that the PeriodicTask and WatcherThread APIs could
be cleaned up further.

>On 2/17/15 5:53 PM, David Holmes wrote: 
>> ---
>>
>> task.cpp
>>
>>  81 int PeriodicTask::time_to_wait() {
>>  82   assert(Thread::current()->is_Watcher_thread(), "must be
>> WatcherThread");
>>
>> This is currently true but not sure it has to be true. If we are
>> assuming/constraining a subset of the task API to only be callable by
>> the WatcherThread then perhaps we should document that in task.hpp ?
>> And as the WatcherThread is a friend, none of those methods need to be
>> public - ie execute_if_pending and time_to_wait (realtime_tick is
>> already private).
>
> I was too focused on adding asserts that enforced how it works today
> and not on how it might be used down the road. There's no reason to
> restrict the caller of time_to_wait() to the WatcherThread. I've
> deleted the assert on line 82 and I added a comment to task.hpp:
>
>    // Requires the PeriodicTask_lock.
>    static int time_to_wait();
>
> By leaving time_to_wait() public, that allows code other than the
> WatcherThread to use it perhaps for debugging...

Okay. I still think the API is somewhat confused about its public interface -
execute_if_pending should be protected for use by WT only (and
subclasses implement it of course). 


Some cleanup was done via JDK-8072439, but another pass through
both PeriodicTask and WatcherThread should be done:

- look at public versus private; don't forget that 'friend' status
  grants access to 'private' methods...
- look at whether the calling thread should be restricted (or not),
  e.g., WatcherThread::stop() should never be called by the
  WatcherThread itself
- enroll() and disenroll() should probably never be called by
  the WatcherThread; what about other non-JavaThreads?

David H. will probably have more ideas here.
Comments
Runtime Triage: This is not on our current list of priorities. We will consider this feature if we receive additional customer requirements.
11-02-2020