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] is harmony-1951 a bug or a feature?
Date Wed, 22 Nov 2006 04:36:10 GMT
Salikh,

What you suggest below make sense for where we are at with Harmony VM at
this point in time.   If at a later date we discover its a performance
bottleneck we can characterize the hotspots and figure out something
different.

I tried to apply your below patch to the current svn HEAD.  It looks like
your local source tree is out of date.  The patch failed.

The do while {...} might actually need to make a call to
hythread_safe_point() somewhere in the loop.  I worry about GC latency.  I
haven't thought about this too carefully.

As far as I can tell, your patch does not address the situation where the
targetted threads appear to miss the interrupt from the main thread.  Am I
correct?


On 11/21/06, Salikh Zakirov <Salikh.Zakirov@intel.com> wrote:
>
> Artem Aliev wrote:
> > Guys,
> >
> > I attach a kind of fix for the problem to the JIRA, but I'm not sure I
> > want to commit such fix.
> > Please review and comment
>
> Thanks to Artem for clear explanation of the behaviour of the Test3.
> (see HARMONY-1951 comments).
>
> Formally speaking, the Java specification does not guarantee that
> Test3.java
> from HARMONY-1951 will work correctly, as specification allows for
> "spurious" wake ups from wait().
>
> Thus, the behaviour of the test is still within compliant with the spec.
>
> Still, I agree that it would be nice to fix interruption() to prevent
> spurious wakeups.
>
> Regarding the fix proposed by Artem, I think it is overly complex,
> and does many things that are not required by the specification.
> I'm thinking about a simpler fix using a state variable in
> HyThreadMonitor.
> (which is conveniently protected by monitor mutex)
> It looks like it fixes the Test3, but I need to test it some more.
>
> diff --git vm/thread/src/thread_native_fat_monitor.c
> vm/thread/src/thread_native_fat_monitor.c
> index 7e812a2..9994c22 100644
> --- vm/thread/src/thread_native_fat_monitor.c
> +++ vm/thread/src/thread_native_fat_monitor.c
> @@ -67,6 +67,7 @@ IDATA VMCALL hythread_monitor_init_with_
>     mon->flags = flags;
>     mon->name  = name;
>     mon->owner = 0;
> +    mon->notify_flag = 0;
>
>         *mon_ptr = mon;
>     return TM_ERROR_NONE;
> @@ -205,8 +206,33 @@ IDATA monitor_wait_impl(hythread_monitor
>     self->state |= TM_THREAD_STATE_IN_MONITOR_WAIT;
>     hymutex_unlock(self->mutex);
>
> -    status =  condvar_wait_impl(mon_ptr->condition, mon_ptr->mutex, ms,
> nano, interruptable);
> -
> +    do {
> +        apr_time_t start;
> +        mon_ptr->notify_flag = 0; // clear flag before waiting
> +        start = apr_time_now();
> +        status = condvar_wait_impl(mon_ptr->condition, mon_ptr->mutex,
> ms, nano, interruptable);
> +        if (status != TM_ERROR_NONE
> +                || mon_ptr->notify_flag || hythread_interrupted(self))
> +            break;
> +        // we should not change ms and nano if both are 0 (meaning "no
> timeout")
> +        if (ms || nano) {
> +            apr_interval_time_t elapsed;
> +            elapsed = apr_time_now() - start; // microseconds
> +            nano -= (IDATA)((elapsed % 1000) * 1000);
> +            if (nano < 0) {
> +                ms -= elapsed/1000 + 1;
> +                nano += 1000000;
> +            } else {
> +                ms -= elapsed/1000;
> +            }
> +            if (ms < 0) {
> +                assert(status == TM_ERROR_NONE);
> +                status = TM_ERROR_TIMEOUT;
> +                break;
> +            }
> +            assert(0 <= nano && nano < 1000000);
> +        }
> +    } while (1);
>     hymutex_lock(self->mutex);
>     self->state &= ~TM_THREAD_STATE_IN_MONITOR_WAIT;
>     self->current_condition = NULL;
> @@ -321,6 +347,7 @@ IDATA VMCALL hythread_monitor_notify_all
>         mon_ptr->notify_flag=1;
>         return TM_ERROR_NONE;
> #else
> +    mon_ptr->notify_flag = 1;
>         return hycond_notify_all(mon_ptr->condition);
> #endif
> }
> @@ -347,6 +374,7 @@ IDATA VMCALL hythread_monitor_notify(hyt
>         mon_ptr->notify_flag=1;
>         return TM_ERROR_NONE;
> #else
> +    mon_ptr->notify_flag = 1;
>         return hycond_notify(mon_ptr->condition);
> #endif
> }
>
>


-- 
Weldon Washburn
Intel Enterprise Solutions Software Division

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