harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Weldon Washburn" <weldon...@gmail.com>
Subject Re: [drlvm][threading] questions about wait_count and notify_flag in thread_native_fat_monitor.c
Date Mon, 27 Nov 2006 20:55:39 GMT
Salikh,

Your comments helped a bunch.  I now understand this code.  It looks
correct.   It would be good if you could put more comments in the patch.
Maybe rephrasing the below would work.   One thing to look out for is a race
condition that allows a thread to miss an intended notify.

Also the "#ifdef NO_COND_VARS" is very distracting.   Morphing notify_flag
to notify_count is a good idea.  But I don't see a reason to attempt to
patch up the NO_COND_VARS use of notify_flag/notify_count.  How about
tossing all the code inside #ifdef NO_COND_VARS?


On 11/27/06, Salikh Zakirov <Salikh.Zakirov@intel.com> wrote:
>
> Weldon, I'm assuming you are reviewing the patch
>
> use-HyThreadMonitor.notify_flag-to-prevent-spurious-interrupt-wakeups.patch(3 kb)
> from HARMONY-1951.
>
> Weldon Washburn wrote:
>
> > It looks like wait_count is never initialized to zero.
> > hythread_monitor_init_with_name()
> > initializes notify_flag to zero.  Thus it seems logical that wait_count
> > would also need to be intialized to zero.
>
> In fact, it is initialized to zero implicitly by the following line
> 50     mon = apr_pcalloc(pool, sizeof(HyThreadMonitor));
>
> "calloc" family of functions zero out the allocated memory.
>
> perhaps, the patch could be refined to remove other extraneous zero
> initializations too.
>
> > 2)
> > wait_count and notify_flag appear to serve the same purpose.  Are both
> > variables neccessary?
>
> I do not know the original purpose of wait_count, it looks like the
> function
>     hythread_monitor_num_waiting()
> was provided for sort of functional completeness.
> As far as I know, there are no users of this function.
>
> And this variable turned out to be handy to track the notify events
> in order to be able to ignore spurios wakeups.
> At any given moment of time (when the monitor lock is acquired), the
> counters
> have following meaning:
>
> wait_count = the number of threads either waiting on the monitor or queued
> to
> acquire monitor lock after wakeup.
>
> notify_flag (which should really be renamed to notify_count) = the number
> of
> notify events generated. It cannot be greater than wait_count to prevent
> piling up notify events according to the java specification.
>
> The need of notify_count comes from the fact that we cannot use wait_count
> in a
> woken up thread to decide if the wakeup was spurious or triggered by
> notify event.
>
> > 3)
> > Why does hythread_monitor_notify_all() do:
> >
> >   mon_ptr->notify_flag = mon_ptr->wait_count;
> >
> > and hythread_monitor_notify() do:
> >
> >   if (mon_ptr->notify_flag < mon_ptr->wait_count)
> >         mon_ptr->notify_flag +=1;
>
> This code ensures that notify_flag (count) never exceeds the value
> of wait_count.
> Woken up thread decreases both wait_count and notify_flag by 1,
> thus preserving the invariant.
>
> > 4)
> > As far as I can tell, setting both wait_count and notify_flag to
> arbitrary
> > values will not impact the proper delivery of Object.notify() or
> > Object.notifyAll().  Is this a correct understanding?
>
> Unfortunately, I cannot quite get what does this mean.
>
> Both wait_count and notify_flag has well-defined semantics inside of
> synchronized section (with monitor lock acquired). Setting them to
> arbitrary
> values can break the system, for example:
> * arbitrary decreasing notify_flag will lead to threads missing notify
> events
> * arbitrary increasing notify_flag may lead to spurious wakeups
> * arbitary decreasing wait_count may lead to notify() calls being ignored
> * arbitrary increasing wait_count may lead to notify() calls piled up and
> affecting threads that executed wait() _after_ notify().
>
>
> > 5)
> > The only impact of these variables is the return value of
> > hythread_monitor_num_waiting().  Is this a correct understanding?
>
> As far as I can see, wait_count has no other use before the discussed
> patch.
>
> > 6)
> > I suspect hythread_monitor_num_waiting() is only used to give someone
> > debugging an idea of how many threads where waiting at a given object at
> a
> > point in the past.  Is this a correct understanding?  The
> > hythread_monitor_num_waiting() return value does not have to be the
> current
> > state of the system but a value of -75688 would probably cause the
> person
> > who is debugging to, well, report a bug in the debugger.
>
> Using wait_count for debugging sounds plausible, though I haven't used it.
>
>


-- 
Weldon Washburn
Intel Enterprise Solutions Software Division

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message