Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8074314

PeriodicTask and WatcherThread APIs need to be cleaned up

    XMLWordPrintable

    Details

    • Type: Enhancement
    • Status: Closed
    • Priority: P4
    • Resolution: Won't Fix
    • Affects Version/s: 9
    • Fix Version/s: tbd
    • Component/s: hotspot
    • Labels:
    • Subcomponent:
    • CPU:
      generic
    • OS:
      generic

      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.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              dcubed Daniel Daugherty
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: