Return-Path: Delivered-To: apmail-incubator-harmony-dev-archive@www.apache.org Received: (qmail 91625 invoked from network); 2 Oct 2006 04:47:48 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 2 Oct 2006 04:47:48 -0000 Received: (qmail 13917 invoked by uid 500); 2 Oct 2006 04:47:47 -0000 Delivered-To: apmail-incubator-harmony-dev-archive@incubator.apache.org Received: (qmail 13237 invoked by uid 500); 2 Oct 2006 04:47:46 -0000 Mailing-List: contact harmony-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: harmony-dev@incubator.apache.org Delivered-To: mailing list harmony-dev@incubator.apache.org Received: (qmail 13226 invoked by uid 99); 2 Oct 2006 04:47:45 -0000 Received: from idunn.apache.osuosl.org (HELO idunn.apache.osuosl.org) (140.211.166.84) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 01 Oct 2006 21:47:45 -0700 Authentication-Results: idunn.apache.osuosl.org header.from=evgueni.brevnov@gmail.com; domainkeys=good X-ASF-Spam-Status: No, hits=0.5 required=5.0 tests=DNS_FROM_RFC_ABUSE DomainKey-Status: good X-DomainKeys: Ecelerity dk_validate implementing draft-delany-domainkeys-base-01 Received: from [66.249.82.236] ([66.249.82.236:54263] helo=wx-out-0506.google.com) by idunn.apache.osuosl.org (ecelerity 2.1.1.8 r(12930)) with ESMTP id C1/60-15566-0F990254 for ; Sun, 01 Oct 2006 21:47:45 -0700 Received: by wx-out-0506.google.com with SMTP id s13so1623498wxc for ; Sun, 01 Oct 2006 21:47:42 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=jsWbWhkx7XhRDI0hmhkbGX9jonmLl9X5JYn3hDeecjmdsvLzbZCCrlqJ3FSCTYTvjwCHJRNdttVFwoqcwiLySshTq1nI8XIHe93FuudFEV/Ug7p1KRNhPX/uhEWLCxijvljW65t/00FxjVuqwRoJd6DqKqr0r2D4c6U4ESVeNco= Received: by 10.90.83.14 with SMTP id g14mr2530788agb; Sun, 01 Oct 2006 21:47:42 -0700 (PDT) Received: by 10.90.105.11 with HTTP; Sun, 1 Oct 2006 21:47:41 -0700 (PDT) Message-ID: Date: Mon, 2 Oct 2006 11:47:42 +0700 From: "Evgueni Brevnov" To: harmony-dev@incubator.apache.org Subject: Re: [drlvm] HARMONY-1582 - invocation API for DRLVM In-Reply-To: <6928c5160609291234i6453467gb5f3403d1d58801c@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <79F58F0A-803A-41CD-B6FF-6E92124BD8AB@pobox.com> <8F465C98-3CC9-41C4-B7D0-278E431CC289@pobox.com> <6928c5160609261024w71006efet3dd613b8b9cd2efc@mail.gmail.com> <6928c5160609291234i6453467gb5f3403d1d58801c@mail.gmail.com> X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N On 9/30/06, Andrey Chernyshev wrote: > On 9/27/06, Evgueni Brevnov wrote: > > On 9/27/06, Andrey Chernyshev wrote: > > > On 9/26/06, Evgueni Brevnov 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 wrote: > > > > > On 9/26/06, Geir Magnusson Jr. wrote: > > > > > > > > > > > > On Sep 26, 2006, at 9:39 AM, Evgueni Brevnov wrote: > > > > > > > > > > > > > On 9/26/06, Geir Magnusson Jr. 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 > > > > > > > -- > 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