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.