On 9/30/06, Andrey Chernyshev <[EMAIL PROTECTED]> wrote:
On 9/27/06, Evgueni Brevnov <[EMAIL PROTECTED]> wrote:
> On 9/27/06, Andrey Chernyshev <[EMAIL PROTECTED]> wrote:
> > On 9/26/06, Evgueni Brevnov <[EMAIL PROTECTED]> 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?
I would expect TM be responsible for launching threads, and VM be
responsible for creating JNIEnv for each new thread. VM may have a
method like "create_JNIEnv()" which TM might be using while building a
new Java thread (or attaching the existing native one). We can extend
the existing vm_attach() function for that purpose for now, such that
it would be returning new JNIenv's.
I did exactly like you suggest here :-) Please look at vm_jthread_atach.
>
>
> >
> > (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.
Looking into the patch, it really seems like you just added a
convenience method which does an allocation of the hythread_lib_t
structure with help of APR. How does that help to solve the multiVM
issue? hythread_lib was a variable in vm_main in the past, but what
prevents you from just making it a part of the appropriate JavaVM
struct?
Yes, I did it. If you look at Global_Env structure you will find it
have a pointer to portlib.
>
> >
> > (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?
OK, then why don't suggest just to rename the hythread_global_lock to
the hythread_lib_lock if you feel this is that important? I guess
this must be more safe if we were introducing one more global lock to
the system.
I agree with you. There is no need to keep both. I just left former
one for not introducing aaditional changes in this patch. Do you
suggest to include that in this patch or do it separatly?
>
> >
> > (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...
Right, it isn't good, we should better think how the daemon-related
functionality could be moved to the Java level.
OK
Thanks
Evgueni
Thanks,
Andrey.
>
> Thanks
> Evgueni
>
> >
> > Thanks,
> > Andrey.
> >
> >
> > >
> > > On 9/26/06, Evgueni Brevnov <[EMAIL PROTECTED]> wrote:
> > > > On 9/26/06, Geir Magnusson Jr. <[EMAIL PROTECTED]> wrote:
> > > > >
> > > > > On Sep 26, 2006, at 9:39 AM, Evgueni Brevnov wrote:
> > > > >
> > > > > > On 9/26/06, Geir Magnusson Jr. <[EMAIL PROTECTED]> 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: [EMAIL PROTECTED]
> > > > > >> For additional commands, e-mail: harmony-dev-
> > > > > >> [EMAIL PROTECTED]
> > > > > >>
> > > > > >>
> > > > > >
> > > > > >
---------------------------------------------------------------------
> > > > > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > > > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > > > > For additional commands, e-mail: [EMAIL PROTECTED]
> > > > > >
> > > > >
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > > > For additional commands, e-mail: [EMAIL PROTECTED]
> > > > >
> > > > >
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > For additional commands, e-mail: [EMAIL PROTECTED]
> > >
> > >
> >
> >
> > --
> > Andrey Chernyshev
> > Intel Middleware Products Division
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
--
Andrey Chernyshev
Intel Middleware Products Division
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
