harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Evgueni Brevnov" <evgueni.brev...@gmail.com>
Subject Re: [drlvm] HARMONY-1582 - invocation API for DRLVM
Date Wed, 27 Sep 2006 06:38:24 GMT
On 9/27/06, Andrey Chernyshev <a.y.chernyshev@gmail.com> wrote:
> On 9/26/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
> > >Alexey Varlamov [26/Sep/06 05:11 AM]
> > >Evgueni, thank you, quite impressive work!
> > >Unfortunately patch is so huge it is hard to understand at once.
> >
> > I understand your concern (about reformatting)... I feel very-very
> > uncomfortable when I study purely formatted code. It always hard to
> > understand some one's code especially if it is hard to read. That was
> > not my intention to fix indenting I just can't study such code and
> > reformat it on the fly.... Sorry, I don't know how to switch my patch
> > to the original formatting.
> >
> > >And what really bothers, is that about half of it is just reformatting :(
> > >Can't we really go without white space changes and renaming of
> > >parameters\local vars???
> > Its definitely much less than the half of the patch.
> >
> > >
> > >Kind request: could you please describe shortly what is done in TM -
> > which >essential changes & enhancements?
> >
> > I think a lot of people are interested in answer to this question. Do
> > you? Let me get a break and I will come up with details....
>
> Right, it seems like invocation API patch does extensive changes to
> the TM design and interfaces. Hopefully you can provide us with a
> summary why it was done
> so.

Sure!

> In the meantime, there are a few things I'd like to understand
> first (I didn't
> look through the whole patch yet though):
>
> (1)
> Why did you have to add JavaVM to jthread_create and jthread_attach
> functions? I couldn't see the use for JavaVM in the TM code, except putting it
> into TLS which can always be done outside of TM if one wishes to do so.
> Ideally, we would have TM to know as much less about the rest of VM as >possible.

Mainly because I'm looking forward to have multi VM in a process
address space. So I tried to do everything having this in mind. Take
jthread_attach function. Originally it had JNIEnv * as its first
argument. According to JNI spec JNIEnv * pointer is valid in the
current thread only! Before thread is not attached it doesn't have its
environment. So it can be a parameter to jthread_attach function. To
be honest I go iniline with original design here.... VM calls
jthread_attach to attach thread....jthread_attach calls vm_attach
(vm_jthread_attach now). We may think about changing that to the
following. VM attaches thread to it self by means of vm_jthread_attach
and then passes created JNIEnv * pointer to jthread_attach. What do
you think?


>
> (2)
> What is the purpose of adding hythread_lib_create and
> hythread_lib_destroy functions? I guess we already have
> hythread_init/hythread_shutdown for the similar purpose...
Multi VM again. Each VM should have its own instance of the library.

>
> (3)
> One more lock is added - hythread_lib_lock. How is that differ from
> the hythread_global_lock that we already have? Each extra lock to the
> system may add more possibilities for deadlocks, as well as can
> negatively impact the scalability (unless some of the existing locks
> are split).
hythread_lib_lock acquires exactly the same lock as
hythread_global_lock. Probably I miss something but we need to be
compatible with IBM threading library now. This library has such
function. That's why I added it. Sounds right?

>
> (4)
> Instead of adding "daemon" attribute to hythread_create, I think the
> better approach would be to move the daemon threads count logic out of
> the hythread level - this level is assumed to know nothing about
> daemon threads in Java, the better place to handle that would be
> jthread level (or even better - Java code).
> Did you need this change to implement the invocation API impl, or, it
> was just a side improvement?

When I first looked at that I thought exactly like you. "daemon"
notion should be applied to java threads only. But I found that you
have daemon attribute into HyThread structure. Moreover you have
hythread_wait_for_all_daemon_threads which get me the feeling that any
hythread can be a daemon. I think for a while whether it is good or
not to have such notion for native threads.....and decides it can be
useful sometimes. Current implementation of InvocationAPI uses daemon
status for java threads only. So lets discuss it...

Thanks
Evgueni

>
> Thanks,
> Andrey.
>
>
> >
> > On 9/26/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
> > > On 9/26/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
> > > >
> > > > On Sep 26, 2006, at 9:39 AM, Evgueni Brevnov wrote:
> > > >
> > > > > On 9/26/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
> > > > >> Wanted to start a new thread with better subject line.
> > > > >>
> > > > >> Evgueni... two things:
> > > > >>
> > > > >> 1) by your own admission, there are problems with the patch,
namely
> > > > >> tests failing.
> > > > > I do belive this is not the patch problems. I think this patch
> > > > > disclose existing problems with condition variables implementation.
> > > > >
> > > > >> You also note that c-unit tests have to be turned off
> > > > >> because they no longer link.
> > > > > There is a hack in current cunit tests. Two methods are stubed
> > > > > vm_attach and vm_detach. That's who linking problem is workarounded.
> > > > > We can't go forward with this....
> > > >
> > > > Who can we badger into fixing this?
> > > >
> > > > >
> > > > >>
> > > > >> 2) Alexey V is hoping for a description of the changes, due to
the
> > > > >> size and mix of reformatting changes.
> > > > >>
> > > > >> I don't mind a little breakage to move forward, but as we're
just
> > > > >> getting DRLVM stable after the launcher change, it might be better
if
> > > > >> we don't have to remove major things like c-unit tests to make
that
> > > > >> forward movement.
> > > > > I believe it is a question of one day or even less to get it fixed
by
> > > > > original test authors. Unfortunately, I'm not familiar with cunit
> > > > > tests internals. Ofcourse I can fix it by myself but need a little
bit
> > > > > more time....
> > > >
> > > > Lets see if we can shame them into fixing it... :)
> > > >
> > > > Shame c-unit test authors!  Shame c-unit test authors!
> > >
> > > Ok, I'm expecting a lot of anger on me form c-unit test authors ...
> > > :-) Sorry, guys...
> > >
> > > >
> > > > (goes off to look for patch...)
> > > >
> > > > geir
> > > >
> > > > >
> > > > > Evgueni
> > > > >>
> > > > >> geir
> > > > >>
> > > > >>
> > > > >> ---------------------------------------------------------------------
> > > > >> Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > > >> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > > >> For additional commands, e-mail: harmony-dev-
> > > > >> help@incubator.apache.org
> > > > >>
> > > > >>
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > > >
> > > >
> > >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
>
> --
> Andrey Chernyshev
> Intel Middleware Products Division
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message