Ok, I go home. I have peace in my mind now :-) Thank you all Evgueni On 10/9/06, Geir Magnusson Jr. wrote: > ok - I just passed a smoke test, gc.PhantomReferenceQueueTest, that was > failing. We should look at that and see why it's so flakey. > > If this works, I'm going to commit this thing so we can move forward. > > We should focus on stablizing the c-unit tests, btw... > > geir > > > Geir Magnusson Jr. wrote: > > > > > > Evgueni Brevnov wrote: > >> Geir, Salikh, > >> > >> cunit tests are very unstable...... that's why we have different > >> results on different platforms. It all tests problems. I already fixed > >> several places but it seems there is much more left. I believe we need > >> to revise them all. The main problem is that sleep(somthing) is used > >> to synchronize different threads. Sometimes there is no synchraniztion > >> at all. I propouse for switching it off until they get stable. > >> Can we do it? > > > > No. They all pass for me now, and I think that they are important as > > they are better than nothing. > > > > I suspected that these were just timing issues - we should just hunker > > down and get them fixed ASAP. We're making progress > > > > geir > > > >> > >> Evgueni > >> > >> On 10/9/06, Evgueni Brevnov wrote: > >>> Could you insert sleep_a_click(); just before line 59 right after > >>> hysem_create(&start, 0, 1): > >>> > >>> Does it help? > >>> > >>> On 10/9/06, Geir Magnusson Jr. wrote: > >>> > [echo] ## Executing C unit test: test_ti_instrum > >>> > [exec] Result: 1 > >>> > [echo] INFO: TEST test_jthread_get_all_threads start > >>> > [echo] ERROR: Assertion '(thread_count)==(i + > >>> > initial_thread_count)' failed at /home/geir/dev/apache/harmony/ > >>> > enhanced/trunk/working_vm/vm/tes > >>> > ts/unit/thread/test_ti_instrum.c:94 > >>> > [echo] Init1: initial_thread_count=1, initial_all_threads_count=1 > >>> > [echo] Init3: initial_thread_count=1, initial_all_threads_count=1 > >>> > [echo] Init3: thread_count=3, all_threads_count=3 > >>> > [echo] INFO: TEST test_jthread_get_all_threads: FAILED > >>> > [echo] INFO: TEST test_jthread_get_thread_count start > >>> > [echo] Init1: initial_thread_count=3, initial_all_threads_count=3 > >>> > [echo] INFO: TEST test_jthread_get_thread_count: PASSED > >>> > [echo] INFO: TEST test_jthread_get_blocked_count start > >>> > [echo] INFO: TEST test_jthread_get_blocked_count: PASSED > >>> > [echo] ## TEST FAILED > >>> > > >>> > > >>> > On Oct 9, 2006, at 12:35 AM, Evgueni Brevnov wrote: > >>> > > >>> > > I put debug printing into test_ti_instrum.c and attached it to JIRA. > >>> > > Could you run it on your machine and send me console output. > >>> > > > >>> > > Evgueni > >>> > > > >>> > > On 10/9/06, Geir Magnusson Jr. wrote: > >>> > >> I keep getting a failure when running the tests - > >>> > >> > >>> > >> test_jthread_get_all-threads failling the assertion at > >>> > >> test_ti_instrum.c:80 > >>> > >> > >>> > >> geir > >>> > >> > >>> > >> On Oct 8, 2006, at 7:19 AM, Evgueni Brevnov wrote: > >>> > >> > >>> > >> > While running cunit on Linux it turned out one test case fails > >>> some > >>> > >> > time. The fix is in tests.final.2.patch. > >>> > >> > > >>> > >> > So the last versions to be committed: > >>> > >> > invocation_api.final.patch > >>> > >> > build.final.2.patch > >>> > >> > tests.final.2.patch > >>> > >> > > >>> > >> > Evgueni > >>> > >> > > >>> > >> > > >>> > >> > On 10/8/06, Evgueni Brevnov wrote: > >>> > >> >> I mahaged to resolve the problem on Linux.... will update > >>> > >> >> build.final.patch with build.final.2.patch in a while > >>> > >> >> > >>> > >> >> On 10/8/06, Evgueni Brevnov wrote: > >>> > >> >> > Hi, > >>> > >> >> > > >>> > >> >> > Oh! Ooh! I did that..... I passed cunit, somke, kernel > >>> tests on > >>> > >> >> > Windows and smoke, kernel tests on Linux. Unfortunately I > >>> > >> failed to > >>> > >> >> > link cunit tests on Linux so far. So I disabled cunit on Linux > >>> > >> >> until > >>> > >> >> > the problem is solved. I believe it is acceptable as short > >>> term > >>> > >> >> > solution. I found several problems in cunit tests. I will come > >>> > >> >> up with > >>> > >> >> > my findings later (not today). > >>> > >> >> > > >>> > >> >> > Use latest patches from HARMONY-1582. They are marked as > >>> final. > >>> > >> >> There > >>> > >> >> > are three patches. One for build module, one for cunit > >>> tests and > >>> > >> >> one > >>> > >> >> > for VM itself. > >>> > >> >> > > >>> > >> >> > Thanks > >>> > >> >> > Evgueni > >>> > >> >> > > >>> > >> >> > > >>> > >> >> > On 10/6/06, Geir Magnusson Jr. wrote: > >>> > >> >> > > Nooooooo! LOL > >>> > >> >> > > > >>> > >> >> > > I'm here waiting - This will unblock a whole bunch of > >>> > >> things :) > >>> > >> >> > > > >>> > >> >> > > Thanks for the effort > >>> > >> >> > > > >>> > >> >> > > Evgueni Brevnov wrote: > >>> > >> >> > > > Geir, > >>> > >> >> > > > > >>> > >> >> > > > That's terrible. We have power outage....servers are > >>> down. I > >>> > >> >> can't > >>> > >> >> > > > send the patches now.... will do it tomorrow > >>> > >> >> > > > > >>> > >> >> > > > Evgueni > >>> > >> >> > > > On 10/5/06, Geir Magnusson Jr. wrote: > >>> > >> >> > > >> woo hoo! here we go... > >>> > >> >> > > >> > >>> > >> >> > > >> > >>> > >> >> > > >> Andrey Chernyshev wrote: > >>> > >> >> > > >> > Hi Evgueni, > >>> > >> >> > > >> > > >>> > >> >> > > >> > On 10/4/06, Evgueni Brevnov > >>> > >> >> wrote: > >>> > >> >> > > >> >> Hi All, > >>> > >> >> > > >> >> > >>> > >> >> > > >> >> I have attached updated patch to the JIRA. It should > >>> > >> >> resolve remain > >>> > >> >> > > >> >> concerns. Andrey, could you give a green light now? > >>> > >> >> > > >> > > >>> > >> >> > > >> > Thanks for updating the patch! I agree it it can be > >>> > >> >> committed, at > >>> > >> >> > > >> > least signatures look good. 5 revisions seem like more > >>> > >> >> than enough :). > >>> > >> >> > > >> > Anyways, I think the remaining issues can be resolved > >>> > >> >> with additional > >>> > >> >> > > >> > patches. Thanks again for the good work and your > >>> > >> patience. > >>> > >> >> > > >> > > >>> > >> >> > > >> > Thanks, > >>> > >> >> > > >> > Andrey. > >>> > >> >> > > >> > > >>> > >> >> > > >> >> > >>> > >> >> > > >> >> Thanks > >>> > >> >> > > >> >> Evgueni > >>> > >> >> > > >> >> > >>> > >> >> > > >> >> On 10/4/06, Evgueni Brevnov > >>> > >>> > >> >> wrote: > >>> > >> >> > > >> >> > Andrey, > >>> > >> >> > > >> >> > > >>> > >> >> > > >> >> > I see your points. I think both approaches have > >>> > >> >> advantages and > >>> > >> >> > > >> >> > disadvantages. I think it is quite hard to say which > >>> > >> >> approach is > >>> > >> >> > > >> >> > better until we play with one VM only. I still feel > >>> > >> >> like introducing > >>> > >> >> > > >> >> > one more dependence is better than spreading code > >>> > >> >> which deals with > >>> > >> >> > > >> >> > attachment among VM and TM. We really get stuck. OK, > >>> > >> >> just because I > >>> > >> >> > > >> >> > want to move forward I will do required changes to > >>> > >> remove > >>> > >> >> > > >> >> > vm_create_jthread from TM. I believe that will > >>> resolve > >>> > >> >> all our > >>> > >> >> > > >> >> > disagreements and the patch will be applied soon. > >>> > >> >> > > >> >> > > >>> > >> >> > > >> >> > > >>> > >> >> > > >> >> > Thanks > >>> > >> >> > > >> >> > Evgueni. > >>> > >> >> > > >> >> > > >>> > >> >> > > >> >> > On 10/4/06, Andrey Chernyshev > >>> > >> >> wrote: > >>> > >> >> > > >> >> > > On 10/3/06, Evgueni Brevnov > >>> > >> >> wrote: > >>> > >> >> > > >> >> > > > On 10/3/06, Andrey Chernyshev > >>> > >> >> wrote: > >>> > >> >> > > >> >> > > > > On 10/2/06, Evgueni Brevnov > >>> > >> >> wrote: > >>> > >> >> > > >> >> > > > > > Andrey, > >>> > >> >> > > >> >> > > > > > > >>> > >> >> > > >> >> > > > > > Just to be clear.... I agree with you it is > >>> > >> more > >>> > >> >> > > >> convenient if > >>> > >> >> > > >> >> > > > > > jthread_create takes JNIEnv instead of > >>> > >> JavaVM. It > >>> > >> >> > > >> reflects that > >>> > >> >> > > >> >> > > > > > current thread has been attached already. Do > >>> > >> >> you think it > >>> > >> >> > > >> >> makes sense > >>> > >> >> > > >> >> > > > > > to get rid of JNIEnv and use > >>> > >> >> jthread_get_JNI_env in that > >>> > >> >> > > >> case? > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > The jthread_* layer is designed like if it > >>> were > >>> > >> >> a regular JNI > >>> > >> >> > > >> >> > > > > application which is meant to be called > >>> from the > >>> > >> >> Java code, > >>> > >> >> > > >> hence > >>> > >> >> > > >> >> > > > > every function on that layer receives > >>> JNIenv. We > >>> > >> >> can probably > >>> > >> >> > > >> >> get rid > >>> > >> >> > > >> >> > > > > of the JNEnv parameter for jthread_* > >>> functions, > >>> > >> >> assuming that > >>> > >> >> > > >> >> TM can > >>> > >> >> > > >> >> > > > > always extract JNIenv for the current thread > >>> > >> with > >>> > >> >> > > >> >> > > > > jthread_get_JNI_env(). > >>> > >> >> > > >> >> > > > > My only concern would be the performance - > >>> once > >>> > >> >> the JNIenv is > >>> > >> >> > > >> >> already > >>> > >> >> > > >> >> > > > > known for the native part of the kernel > >>> classes > >>> > >> >> impl, it > >>> > >> >> > > >> must be > >>> > >> >> > > >> >> > > > > cheaper to pass JNIEnv to TM as an extra > >>> > >> >> parameter rather than > >>> > >> >> > > >> >> extract > >>> > >> >> > > >> >> > > > > it from the TLS again. > >>> > >> >> > > >> >> > > > > Other than that, I see no strong advantages in > >>> > >> >> keeping JNIEnv > >>> > >> >> > > >> >> parameter. > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > > Regarding jthread_attach. I still didn't get > >>> > >> >> your point.... > >>> > >> >> > > >> >> Clarify it > >>> > >> >> > > >> >> > > > > > please if you think jhread_attach should be > >>> > >> >> modified. > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > Sorry for being not clear: I think for > >>> > >> >> jthread_attach, we have > >>> > >> >> > > >> >> two options: > >>> > >> >> > > >> >> > > > > 1) Make JNIEnv input parameter - it must be > >>> new > >>> > >> >> JNIEnv that VM > >>> > >> >> > > >> >> > > > > pre-allocates for the new Java thread. > >>> > >> >> jthread_attach > >>> > >> >> > > >> would just > >>> > >> >> > > >> >> > > > > associate it with the current thread. > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > 2) Obtain JNIEnv via vm_attach() callback. In > >>> > >> >> this case, if > >>> > >> >> > > >> >> > > > > vm_attach() callback implementation needs to > >>> > >> >> know for which > >>> > >> >> > > >> >> JavaVM new > >>> > >> >> > > >> >> > > > > JNIenv has to be allocated, then we'll need to > >>> > >> >> add JavaVM as > >>> > >> >> > > >> >> input > >>> > >> >> > > >> >> > > > > parameter for jthread_attach(). > >>> > >> >> > > >> >> > > > > I think both options should be fine, (1) would > >>> > >> >> probably > >>> > >> >> > > >> keep TM > >>> > >> >> > > >> >> > > > > interface a bit lighter, though (2) may look > >>> > >> >> more closer to > >>> > >> >> > > >> >> the JNI > >>> > >> >> > > >> >> > > > > invocation API idea. > >>> > >> >> > > >> >> > > > > So I think adding JavaVM specifically for > >>> > >> >> jthread_attach > >>> > >> >> > > >> may make > >>> > >> >> > > >> >> > > > > sense given the explanation you provided > >>> > >> earlier. > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > The concern I would have regarding the > >>> proposed > >>> > >> >> jthread_attach > >>> > >> >> > > >> >> > > > > implementation is a call to > >>> vm_create_jthread() > >>> > >> >> - this call > >>> > >> >> > > >> >> introduces > >>> > >> >> > > >> >> > > > > an extra dependency of TM on vmcore that I'd > >>> > >> >> prefer to be > >>> > >> >> > > >> >> avoided. In > >>> > >> >> > > >> >> > > > > the original version, jthread_attach() was > >>> > >> >> taking jthread > >>> > >> >> > > >> >> argument of > >>> > >> >> > > >> >> > > > > the already prepared j.l.Thread. > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > I understand your concern. Unfortunately I don't > >>> > >> >> see what we > >>> > >> >> > > >> can do > >>> > >> >> > > >> >> > > > here. Let me explain. In the beginning you > >>> have an > >>> > >> >> unattached > >>> > >> >> > > >> >> native > >>> > >> >> > > >> >> > > > thread. To be able to call java code (which is > >>> > >> >> required for > >>> > >> >> > > >> >> > > > constructing j.l.Thread instance) the thread > >>> > >> >> should be attached > >>> > >> >> > > >> >> first. > >>> > >> >> > > >> >> > > > To be specific it should be attached to VM by > >>> > >> >> calling vm_attach > >>> > >> >> > > >> >> which > >>> > >> >> > > >> >> > > > will return a valid JNIEnv It is valid to use > >>> JNI > >>> > >> >> from that > >>> > >> >> > > >> moment. > >>> > >> >> > > >> >> > > > Obtained JNIEnv can be used to execute java code > >>> > >> >> and create > >>> > >> >> > > >> >> j.l.Thread > >>> > >> >> > > >> >> > > > instance. Since we do vm_attach in > >>> jthread_attach > >>> > >> >> we need to do > >>> > >> >> > > >> >> > > > vm_create_jthread inside jthread_atach as well. > >>> > >> >> > > >> >> > > > Of course it can be an alternative to do > >>> vm_attach > >>> > >> >> and > >>> > >> >> > > >> >> > > > vm_create_jthread outside of TM right before > >>> > >> >> jthread_attach. > >>> > >> >> > > >> >> Sure it > >>> > >> >> > > >> >> > > > will reduce one dependence between VM and TM. > >>> But > >>> > >> >> it seems like > >>> > >> >> > > >> >> > > > artificial action for me just because of > >>> > >> >> dependency.... > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > Why do you think it is artificial? I would rather > >>> > >> >> prefer not to > >>> > >> >> > > >> throw > >>> > >> >> > > >> >> > > vm_attach event from the jthread_attach() call at > >>> > >> >> all than to add > >>> > >> >> > > >> >> > > extra dependency. The idea of jthread layer is a > >>> > >> >> Java face to > >>> > >> >> > > >> >> > > hythread, it is meant to know either a little or > >>> > >> >> nothing about the > >>> > >> >> > > >> >> > > rest of VM. It may be logical to throw vm_attach > >>> > >> >> call from the > >>> > >> >> > > >> newly > >>> > >> >> > > >> >> > > created thread, because there is no other way > >>> to let > >>> > >> >> know VM > >>> > >> >> > > >> the new > >>> > >> >> > > >> >> > > thread is created. VM attach is a different case - > >>> > >> >> VM already > >>> > >> >> > > >> knows > >>> > >> >> > > >> >> > > about new Java thread at the time of the > >>> > >> >> AttachCurrentThread call. > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > > Do you think it makes sense to replace a > >>> single > >>> > >> >> jthread > >>> > >> >> > > >> >> parameter with > >>> > >> >> > > >> >> > > > > a variety of stuff (like thread group, > >>> name)? It > >>> > >> >> seems the > >>> > >> >> > > >> only > >>> > >> >> > > >> >> > > > > purpose of at these args is to be passed > >>> back to > >>> > >> >> VM for > >>> > >> >> > > >> >> > > > > vm_jthread_create(). I would suggest to change > >>> > >> >> this and try > >>> > >> >> > > >> doing > >>> > >> >> > > >> >> > > > > either of: > >>> > >> >> > > >> >> > > > > 1) Make signature of jthread_attach with 3 > >>> args, > >>> > >> >> JavaVM, > >>> > >> >> > > >> >> jthread and the daemon. > >>> > >> >> > > >> >> > > > Why do you want to pass daemon to TM but > >>> thread's > >>> > >> >> name and > >>> > >> >> > > >> >> group. Just > >>> > >> >> > > >> >> > > > because current TM doesn't need such > >>> information? > >>> > >> >> What if it is > >>> > >> >> > > >> >> > > > required to get thread name later? Say by > >>> > >> introducing > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > I imagine you need a daemon attribute since the TM > >>> > >> >> is still > >>> > >> >> > > >> managing > >>> > >> >> > > >> >> > > the thread daemonality. TM is not managing thread > >>> > >> >> name and group - > >>> > >> >> > > >> >> > > they are all kept on the Java level, hence passing > >>> > >> >> them may be > >>> > >> >> > > >> >> > > excessive. > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > > jthread_get_name... What will you do in that > >>> case? > >>> > >> >> Let me > >>> > >> >> > > >> guess you > >>> > >> >> > > >> >> > > > will call j.l.Thread.getName. Right. Ok! In that > >>> > >> >> case I see no > >>> > >> >> > > >> >> > > > problems to call j.l.Thread.isDaemon. Do you > >>> > >> >> agree? So it > >>> > >> >> > > >> doesn't > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > As I suggested earlier, the best way to handle > >>> > >> >> daemonality would > >>> > >> >> > > >> >> > > probably be in pure Java - in j.l.Thread (or > >>> > >> >> > > >> j.l.VMThreadManager) or > >>> > >> >> > > >> >> > > whatever. You already lifted it up to the jthread > >>> > >> >> level, but > >>> > >> >> > > >> what if > >>> > >> >> > > >> >> > > we can go a little bit higher... > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > > seems to be a good design to pass only part > >>> of the > >>> > >> >> > > >> information to > >>> > >> >> > > >> >> > > > jthread_atach. Lets look at the signature of > >>> > >> >> > > >> >> AttachCurrentThread. It > >>> > >> >> > > >> >> > > > takes exactly these three parameters (daemon, > >>> > >> >> name, group) > >>> > >> >> > > >> >> passed as a > >>> > >> >> > > >> >> > > > structure. I was thinking about having > >>> exactly the > >>> > >> >> same > >>> > >> >> > > >> >> structure as > >>> > >> >> > > >> >> > > > third parameter of jthread_attach but it occured > >>> > >> >> to be more > >>> > >> >> > > >> >> convinient > >>> > >> >> > > >> >> > > > to pass them seperatly. Although I don't see > >>> > >> >> strong reasons > >>> > >> >> > > >> against > >>> > >> >> > > >> >> > > > having a structure a third parameter. > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > I see. Acually, I would love to keep the > >>> > >> >> jthread_attach func-ty at > >>> > >> >> > > >> >> the > >>> > >> >> > > >> >> > > minimum level which will be needed to handle the > >>> > >> >> only data that TM > >>> > >> >> > > >> >> > > should be aware of. We need a formal boundary > >>> > >> >> between jthread > >>> > >> >> > > >> layer > >>> > >> >> > > >> >> > > and vmcore (otherwise jthread won't have a much of > >>> > >> >> sense, one may > >>> > >> >> > > >> >> > > consider it just as a convenience set of functions > >>> > >> >> in vmcore which > >>> > >> >> > > >> >> are > >>> > >> >> > > >> >> > > doing something with threading). > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > > 2) Move the implementation of > >>> vm_create_jtrhead > >>> > >> >> () to > >>> > >> >> > > >> >> > > > > thread_java_basic.c - could it be written in > >>> > >> >> pure JNI without > >>> > >> >> > > >> >> using > >>> > >> >> > > >> >> > > > > internal VM API like class_alloc_new_object()? > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > Yes, this can be done but it doesn't fix the > >>> > >> >> problem. You still > >>> > >> >> > > >> >> need > >>> > >> >> > > >> >> > > > to know about internal constructor of j.l.Thread > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > That's bad. Given what you said, now it seems that > >>> > >> >> the most > >>> > >> >> > > >> >> preferable > >>> > >> >> > > >> >> > > sequence for AttachCurrentThread impl still > >>> would be > >>> > >> >> like: > >>> > >> >> > > >> >> > > JNIEnv = vm_attach(); > >>> > >> >> > > >> >> > > jthread = create_jthread(JNIenv) > >>> > >> >> > > >> >> > > jthread_attach(JNIEnv, jthread) // stores > >>> JNIEnv and > >>> > >> >> Hythread into > >>> > >> >> > > >> >> > > TLS, doesn't call vm_attach any longer. > >>> > >> >> > > >> >> > > - this is almost what we had from the beginning... > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > Thanks, > >>> > >> >> > > >> >> > > Andrey. > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > Thanks > >>> > >> >> > > >> >> > > > Evgueni > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > Thanks, > >>> > >> >> > > >> >> > > > > Andrey. > >>> > >> >> > > >> >> > > > > > >>> > >> >> > > >> >> > > > > > > >>> > >> >> > > >> >> > > > > > Thank you > >>> > >> >> > > >> >> > > > > > Evgueni > >>> > >> >> > > >> >> > > > > > > >>> > >> >> > > >> >> > > > > > On 10/2/06, Evgueni Brevnov > >>> > >> >> > >>> > >> >> > > >> wrote: > >>> > >> >> > > >> >> > > > > > > On 9/29/06, Andrey Chernyshev > >>> > >> >> > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > On 9/29/06, Evgueni Brevnov > >>> > >> >> > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > Artem, > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > Thank you for your feedback.... > >>> find my > >>> > >> >> inlined..... > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > On 9/29/06, Artem Aliev > >>> > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > Evgueni, > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > I got most of your changes, but > >>> still > >>> > >> >> disagree > >>> > >> >> > > >> with all > >>> > >> >> > > >> >> > > > > > > > > > hythread/jthread interface changes. > >>> > >> >> Could leave > >>> > >> >> > > >> >> interface unchanged. > >>> > >> >> > > >> >> > > > > > > > > > See following possible solutions, > >>> that > >>> > >> >> could solve > >>> > >> >> > > >> >> the same problems > >>> > >> >> > > >> >> > > > > > > > > > without interface changes. > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > 1) daemon attribute is a java > >>> > >> >> specific. (Andrey > >>> > >> >> > > >> >> mentioned this already). > >>> > >> >> > > >> >> > > > > > > > > > Could you please move it back. to > >>> > >> >> the jthread > >>> > >> >> > > >> >> layer. It is better > >>> > >> >> > > >> >> > > > > > > > > > to rename > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> hythread_wait_for_all_nondaemon_threads() to > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> jthread_wait_for_all_nondaemon_threads(). > >>> > >> >> > > >> >> > > > > > > > > Ok, I see no problems to move "daemon" > >>> > >> >> to java layer. > >>> > >> >> > > >> >> In that case: > >>> > >> >> > > >> >> > > > > > > > > 1) I will move > >>> > >> >> > > >> >> hythread_wait_for_all_nondaemon_threads() from > >>> > >> >> > > >> >> > > > > > > > > thread_init.c to one which implements > >>> > >> >> java layer. > >>> > >> >> > > >> >> > > > > > > > > 2) I will move daemon field from > >>> > >> >> HyThread structure. > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > Agree? > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > Sounds good to me. > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > OK, will do that. > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > 2) JavaVM could be retrieved from > >>> > >> >> JNIEnv by > >>> > >> >> > > >> >> jni_get_java_vm(). So > >>> > >> >> > > >> >> > > > > > > > > > let the jthread_create() and > >>> others to > >>> > >> >> use JNIEnv > >>> > >> >> > > >> >> (that is passed from > >>> > >> >> > > >> >> > > > > > > > > > java native method). > >>> > >> >> > > >> >> > > > > > > > > > The vm_attach could get old JNI env > >>> > >> >> and create new > >>> > >> >> > > >> >> one for the new thread. > >>> > >> >> > > >> >> > > > > > > > > > The first JNIEnv is created in > >>> > >> >> CreateVM call and > >>> > >> >> > > >> >> could be passed to > >>> > >> >> > > >> >> > > > > > > > > > the first thread at startup. > >>> > >> >> > > >> >> > > > > > > > > No, no, no. I completely disagree with > >>> > >> >> that!!! Why do > >>> > >> >> > > >> >> you like JNIEnv > >>> > >> >> > > >> >> > > > > > > > > but JavaVM. Why do you think that > >>> > >> >> passing JavaVM > >>> > >> >> > > >> >> instead of JNIEnv > >>> > >> >> > > >> >> > > > > > > > > makes TM less modular? I don't see any > >>> > >> >> difference > >>> > >> >> > > >> >> here.... It seems > >>> > >> >> > > >> >> > > > > > > > > for me like a big big hack to grab > >>> > >> >> JNIEnv from another > >>> > >> >> > > >> >> thread and pass > >>> > >> >> > > >> >> > > > > > > > > it to jthread_attach to perform > >>> > >> >> operations in the > >>> > >> >> > > >> >> current thread. > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > TM needs to know JNIEnv, mainly for > >>> > >> >> managing the > >>> > >> >> > > >> >> references to > >>> > >> >> > > >> >> > > > > > > > objects, throwing exceptions and calling > >>> > >> >> run() method of > >>> > >> >> > > >> >> a new thread. > >>> > >> >> > > >> >> > > > > > > > JNI spec proposes that JNIEnv is valid > >>> > >> >> within the given > >>> > >> >> > > >> >> thread, this > >>> > >> >> > > >> >> > > > > > > > is why TM holds the JNIEnv pointer at > >>> the > >>> > >> >> moment. This > >>> > >> >> > > >> >> is why TM likes > >>> > >> >> > > >> >> > > > > > > > the JNIEnv. > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > Having the JNIEnv, one is able to get > >>> > >> >> JavaVM but not > >>> > >> >> > > >> >> vise versa. This > >>> > >> >> > > >> >> > > > > > > > is why TM doesn't like the JavaVM :) > >>> > >> >> > > >> >> > > > > > > I see your point. Only one note this is > >>> true > >>> > >> >> for already > >>> > >> >> > > >> >> attached threads... > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > I agree with you that there is a design > >>> > >> >> flaw that the > >>> > >> >> > > >> >> JNIEnv is copied > >>> > >> >> > > >> >> > > > > > > > from the parent thread to a child thread > >>> > >> >> during thread > >>> > >> >> > > >> >> creation. I > >>> > >> >> > > >> >> > > > > > > > think it could be resolved via > >>> vm_attach() > >>> > >> >> hook - with > >>> > >> >> > > >> >> this event, VM > >>> > >> >> > > >> >> > > > > > > > might tell the TM what the JNIEnv > >>> pointer > >>> > >> >> for new thread > >>> > >> >> > > >> >> should be. I > >>> > >> >> > > >> >> > > > > > > > think you did that by extending the > >>> > >> >> vm_attach() call > >>> > >> >> > > >> >> with the JNIEnv** > >>> > >> >> > > >> >> > > > > > > > argument. > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > What is not completely clear is, why do > >>> > >> >> you have to pass > >>> > >> >> > > >> >> the JavaVM > >>> > >> >> > > >> >> > > > > > > > forth and back, once from VM to TM, and > >>> > >> >> then back from > >>> > >> >> > > >> >> TM to VM again? > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > If you need to know in jthread_attach, > >>> > >> >> which particular > >>> > >> >> > > >> >> VM vm_attach() > >>> > >> >> > > >> >> > > > > > > > event is coming from, you could have > >>> > >> >> vm_attach like > >>> > >> >> > > >> >> > > > > > > > vm_attach(JNIEnv* currentThreadEnv, > >>> > >> JNIEnv** > >>> > >> >> > > >> >> newThreadEnv). > >>> > >> >> > > >> >> > > > > > > I'm a little bit confused.....Current > >>> thread > >>> > >> >> hasn't been > >>> > >> >> > > >> >> attached yet. > >>> > >> >> > > >> >> > > > > > > So there is no JNIEnv for it yet. How > >>> it can > >>> > >> >> be passed as > >>> > >> >> > > >> >> the first > >>> > >> >> > > >> >> > > > > > > parameter to vm_attach()? > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > > Then you will be always able to get the > >>> > >> >> JavaVM from the > >>> > >> >> > > >> >> JNIEnv. > >>> > >> >> > > >> >> > > > > > > > The only difference is that you are > >>> > >> >> currently doing > >>> > >> >> > > >> >> JNIEnv->JavaVM > >>> > >> >> > > >> >> > > > > > > > conversion in the VMThreadManager, > >>> but why > >>> > >> >> can't you > >>> > >> >> > > >> >> just do this in > >>> > >> >> > > >> >> > > > > > > > vm_attach() without changing the > >>> interface > >>> > >> >> of the TM? > >>> > >> >> > > >> >> > > > > > > > So far JavaVM really looks like an extra > >>> > >> >> knowledge that > >>> > >> >> > > >> >> TM doesn't > >>> > >> >> > > >> >> > > > > > > > have to be aware of. > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > Moreover there is no JNIEnv when main > >>> > >> >> thread attaches > >>> > >> >> > > >> >> to VM. So we > >>> > >> >> > > >> >> > > > > > > > > should either keep it as is or change > >>> > >> >> original design > >>> > >> >> > > >> >> of TM and attach > >>> > >> >> > > >> >> > > > > > > > > thread to VM before attaching it to > >>> TM. > >>> > >> >> In that case > >>> > >> >> > > >> >> we will have > >>> > >> >> > > >> >> > > > > > > > > valid JNIEnv which can be passed to > >>> > >> >> jthread_atatch. We > >>> > >> >> > > >> >> need to think > >>> > >> >> > > >> >> > > > > > > > > over it twice before changing > >>> > >> something.... > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > Right. For jthread_attach, JNIenv > >>> needs to > >>> > >> >> be changed > >>> > >> >> > > >> >> from being input > >>> > >> >> > > >> >> > > > > > > > parameter to being the output parameter. > >>> > >> >> The way how > >>> > >> >> > > >> >> JNIenv is > >>> > >> >> > > >> >> > > > > > > > obtained by TM should be vm_attach() > >>> > >> event. > >>> > >> >> > > >> >> > > > > > > OK, JNIEnv is output parameter. And it > >>> > >> >> obtained by > >>> > >> >> > > >> >> vm_attach(). This > >>> > >> >> > > >> >> > > > > > > is exactly like I do in the patch. What I > >>> > >> >> don't understand > >>> > >> >> > > >> >> is how > >>> > >> >> > > >> >> > > > > > > jthread_attach knows to which VM the > >>> thread > >>> > >> >> should be > >>> > >> >> > > >> >> attached? Do you > >>> > >> >> > > >> >> > > > > > > suggest calling vm_attach first to create > >>> > >> >> JNIEnv it pass > >>> > >> >> > > >> >> it to > >>> > >> >> > > >> >> > > > > > > jthread_attach? > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > 4) Original classlib hythread do > >>> > >> not use > >>> > >> >> > > >> >> hythread_library_t in arguments, > >>> > >> >> > > >> >> > > > > > > > > > It uses following code: > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > hythread_library_t lib = > >>> GLOBAL_DATA > >>> > >> >> > > >> >> (default_library); > >>> > >> >> > > >> >> > > > > > > > > > or > >>> > >> >> > > >> >> > > > > > > > > > hythread_library_t library = > >>> thread- > >>> > >> >> >library; > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > So could you please use the same > >>> > >> >> mechanism and > >>> > >> >> > > >> >> remove hythread_*_ex >functions. > >>> > >> >> > > >> >> > > > > > > > > Let's see if classlib's hythread needs > >>> > >> >> changing first. > >>> > >> >> > > >> >> It seems for me > >>> > >> >> > > >> >> > > > > > > > > such code prevents us from having > >>> > >> multi VM. > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > 5. You introduce more then one jni > >>> > >> >> env, but still > >>> > >> >> > > >> >> use global variable for it. > >>> > >> >> > > >> >> > > > > > > > > > So all changes like following: > >>> > >> >> > > >> >> > > > > > > > > > - JNIEnv *jenv = (JNIEnv*) > >>> > >> >> jni_native_intf; > >>> > >> >> > > >> >> > > > > > > > > > + JNIEnv *jenv = jni_native_intf; > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > should be like: > >>> > >> >> > > >> >> > > > > > > > > > - JNIEnv *jenv = (JNIEnv*) > >>> > >> >> jni_native_intf; > >>> > >> >> > > >> >> > > > > > > > > > + JNIEnv *jenv = get_jni_env > >>> > >> >> (jthread_self()); > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > Ok, I see. I agree that global > >>> > >> >> jni_native_intf should > >>> > >> >> > > >> >> not be used. > >>> > >> >> > > >> >> > > > > > > > > There was simple reason why I altered > >>> > >> >> such lines. > >>> > >> >> > > >> >> Because I changed > >>> > >> >> > > >> >> > > > > > > > > the type of jni_native_intf and no > >>> > >> >> casting operator > >>> > >> >> > > >> >> is needed now. To > >>> > >> >> > > >> >> > > > > > > > > be honest I think get_jni_env > >>> > >> >> (jthread_self()) can be > >>> > >> >> > > >> >> good as temporary > >>> > >> >> > > >> >> > > > > > > > > solution only. Lets wait for design of > >>> > >> >> multi VM and > >>> > >> >> > > >> >> fix it according > >>> > >> >> > > >> >> > > > > > > > > to it. > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > While we are in JNI code, we always have > >>> > >> >> the JNIenv (at > >>> > >> >> > > >> >> least > >>> > >> >> > > >> >> > > > > > > > initially it comes from Java code). > >>> If we > >>> > >> >> consider VM > >>> > >> >> > > >> >> code as if it > >>> > >> >> > > >> >> > > > > > > > was a JNI application, then it seems > >>> like > >>> > >> >> we should be > >>> > >> >> > > >> >> just passing > >>> > >> >> > > >> >> > > > > > > > JNIEnv as a parameter to all > >>> functions in > >>> > >> >> VM. Or, we can > >>> > >> >> > > >> >> be taking it > >>> > >> >> > > >> >> > > > > > > > from TLS (via jthread_self()), depending > >>> > >> >> on which way is > >>> > >> >> > > >> >> faster... > >>> > >> >> > > >> >> > > > > > > Agree. > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > 6). And small remarks: > >>> > >> >> > > >> >> > > > > > > > > > +jint vm_init1(JavaVM_Internal * > >>> > >> java_vm, > >>> > >> >> > > >> >> JavaVMInitArgs * vm_arguments); > >>> > >> >> > > >> >> > > > > > > > > > +jint vm_init2(JNIEnv_Internal * > >>> > >> >> jni_env); > >>> > >> >> > > >> >> > > > > > > > > > Could you make names more > >>> meaningful, > >>> > >> >> then 1,2,3...? > >>> > >> >> > > >> >> > > > > > > > > Ok, will do that. > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > class VM_thread { > >>> > >> >> > > >> >> > > > > > > > > > ... > >>> > >> >> > > >> >> > > > > > > > > > + JNIEnv_Internal * jni_env; > >>> > >> >> > > >> >> > > > > > > > > > The jthread already has the jni_env > >>> > >> >> pointer, you do > >>> > >> >> > > >> >> not need to > >>> > >> >> > > >> >> > > > > > > > > > duplicate it here. > >>> > >> >> > > >> >> > > > > > > > > > forexample by using > >>> > >> >> > > >> >> jthread_get_JNI_env(jthread_self()); > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > Yes I know. I don't see any problems > >>> > >> >> here. Some times > >>> > >> >> > > >> >> it is much more > >>> > >> >> > > >> >> > > > > > > > > convenient to get JNIEnv from > >>> VM_thread > >>> > >> >> structure (and > >>> > >> >> > > >> >> faster) instead > >>> > >> >> > > >> >> > > > > > > > > of doing jthread_get_JNI_env > >>> > >> (jthread_self > >>> > >> >> ()). So I > >>> > >> >> > > >> >> need strong > >>> > >> >> > > >> >> > > > > > > > > arguments for removing it. Again it > >>> > >> >> seems that should > >>> > >> >> > > >> >> be addressed in > >>> > >> >> > > >> >> > > > > > > > > design of multi VM. So lets forget > >>> about > >>> > >> >> it for a > >>> > >> >> > > >> >> while... > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > I think that the data duplication would > >>> > >> >> always serve as > >>> > >> >> > > >> >> a potential > >>> > >> >> > > >> >> > > > > > > > source of errors - while updating one > >>> copy > >>> > >> >> of object, > >>> > >> >> > > >> >> you may forget > >>> > >> >> > > >> >> > > > > > > > to update the other, often resulting > >>> into > >>> > >> >> a strange > >>> > >> >> > > >> >> behavior of the > >>> > >> >> > > >> >> > > > > > > > whole application. Let's see what are > >>> the > >>> > >> >> specific > >>> > >> >> > > >> >> performance > >>> > >> >> > > >> >> > > > > > > > concerns that have to be addressed. > >>> To get > >>> > >> >> VM_thread > >>> > >> >> > > >> >> structure, you > >>> > >> >> > > >> >> > > > > > > > would eventually go to the TLS, just > >>> like > >>> > >> >> > > >> >> > > > > > > > jthread_get_JNI_env(jthread_self() > >>> > >> would do. > >>> > >> >> > > >> >> > > > > > > If there is already VM_thread structure > >>> for > >>> > >> >> some reasons > >>> > >> >> > > >> >> then there > >>> > >> >> > > >> >> > > > > > > will be no extra access to TLS. It is > >>> > >> >> definitely much > >>> > >> >> > > >> more in > >>> > >> >> > > >> >> > > > > > > jthread_get_JNI_env(jthread_self() than > >>> just > >>> > >> >> one TLS > >>> > >> >> > > >> >> access and one > >>> > >> >> > > >> >> > > > > > > dereferncing. I don't think it is a really > >>> > >> >> big problem > >>> > >> >> > > >> >> now. Do you > >>> > >> >> > > >> >> > > > > > > agree to look at this later. I guess > >>> > >> multi VM > >>> > >> >> > > >> >> implementation will > >>> > >> >> > > >> >> > > > > > > alter it in any case. > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > Thanks > >>> > >> >> > > >> >> > > > > > > Evgueni > >>> > >> >> > > >> >> > > > > > > > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > Thanks, > >>> > >> >> > > >> >> > > > > > > > Andrey. > >>> > >> >> > > >> >> > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > Evgueni > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > Thanks > >>> > >> >> > > >> >> > > > > > > > > > Artem > >>> > >> >> > > >> >> > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > On 9/28/06, Evgueni Brevnov > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > > I suppose two days silence means > >>> > >> >> that there is no > >>> > >> >> > > >> >> objects (maybe > >>> > >> >> > > >> >> > > > > > > > > > > interest) against proposed > >>> patch. I > >>> > >> >> would suggest > >>> > >> >> > > >> >> to commit it ASAP. > >>> > >> >> > > >> >> > > > > > > > > > > It really works! There are some > >>> > >> >> cases when current > >>> > >> >> > > >> >> VM crashes but the > >>> > >> >> > > >> >> > > > > > > > > > > patch fixes it. I can work on > >>> > >> >> bringing cunit tests > >>> > >> >> > > >> >> to live as soon as > >>> > >> >> > > >> >> > > > > > > > > > > the patch is committed.... This is > >>> > >> >> just my > >>> > >> >> > > >> >> understanding. > >>> > >> >> > > >> >> > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > Thanks > >>> > >> >> > > >> >> > > > > > > > > > > Evgueni > >>> > >> >> > > >> >> > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > On 9/28/06, Geir Magnusson Jr. > >>> > >> >> > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > > > So where are we here? > >>> > >> >> > > >> >> > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > On Sep 28, 2006, at 12:41 AM, > >>> > >> >> Evgueni Brevnov > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > > On 9/28/06, Weldon Washburn > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > > > >> On 9/26/06, Evgueni Brevnov > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > > > >> > > >>> > >> >> > > >> >> > > > > > > > > > > > >> > On 9/27/06, Andrey > >>> Chernyshev > >>> > >> >> > > >> >> wrote: > >>> > >> >> > > >> >> > > > > > > > > > > > >> > > (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? > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> Well, this sort of, kind of > >>> > >> >> sounds right but > >>> > >> >> > > >> >> not quite. Its a > >>> > >> >> > > >> >> > > > > > > > > > > > >> little more > >>> > >> >> > > >> >> > > > > > > > > > > > >> subtle than being compatible > >>> > >> >> with IBM > >>> > >> >> > > >> >> threading library. The > >>> > >> >> > > >> >> > > > > > > > > > > > >> first goal is > >>> > >> >> > > >> >> > > > > > > > > > > > >> to identify the parts of IBM > >>> > >> >> threading > >>> > >> >> > > >> >> library that are JVM > >>> > >> >> > > >> >> > > > > > > > > > > > >> independent. It > >>> > >> >> > > >> >> > > > > > > > > > > > >> makes sense for DRLVM to be > >>> > >> >> compatible with > >>> > >> >> > > >> >> the independent > >>> > >> >> > > >> >> > > > > > > > > > > > >> parts. This > >>> > >> >> > > >> >> > > > > > > > > > > > >> should be a nobrainer. > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> The parts of IBM threading > >>> > >> >> library that > >>> > >> >> > > >> >> assume a specific JVM > >>> > >> >> > > >> >> > > > > > > > > > > > >> implementation > >>> > >> >> > > >> >> > > > > > > > > > > > >> will be a problem. We will > >>> > >> >> need to find a > >>> > >> >> > > >> >> solution that is > >>> > >> >> > > >> >> > > > > > > > > > > > >> endorsed by all > >>> > >> >> > > >> >> > > > > > > > > > > > >> the stakeholders > >>> (including J9 > >>> > >> >> folks). The > >>> > >> >> > > >> >> hythread_global_lock > >>> > >> >> > > >> >> > > > > > > > > > > > >> falls into > >>> > >> >> > > >> >> > > > > > > > > > > > >> this category. For starts, I > >>> > >> >> would like to > >>> > >> >> > > >> >> see a concise > >>> > >> >> > > >> >> > > > > > > > > > > > >> description from > >>> > >> >> > > >> >> > > > > > > > > > > > >> the portlib owners on what > >>> > >> >> > > >> >> hythread_global_lock protects, which > >>> > >> >> > > >> >> > > > > > > > > > > > >> locks have > >>> > >> >> > > >> >> > > > > > > > > > > > >> to be held before grabbing > >>> this > >>> > >> >> lock, are > >>> > >> >> > > >> >> there any restrictions > >>> > >> >> > > >> >> > > > > > > > > > > > >> on what > >>> > >> >> > > >> >> > > > > > > > > > > > >> system calls can be made > >>> while > >>> > >> >> holding this > >>> > >> >> > > >> >> lock (like sleep or > >>> > >> >> > > >> >> > > > > > > > > > > > >> wait), etc. > >>> > >> >> > > >> >> > > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > > Weldon, I completely agree > >>> with > >>> > >> >> what your are > >>> > >> >> > > >> >> saying. It's common > >>> > >> >> > > >> >> > > > > > > > > > > > > problem of current hythread > >>> that > >>> > >> >> should be > >>> > >> >> > > >> >> resolved some how. I just > >>> > >> >> > > >> >> > > > > > > > > > > > > go inline with current > >>> > >> >> implementation and > >>> > >> >> > > >> >> added two missing functions. > >>> > >> >> > > >> >> > > > > > > > > > > > > Missing these can lead to the > >>> > >> >> same problems as > >>> > >> >> > > >> >> with hythread_exit > >>> > >> >> > > >> >> > > > > > > > > > > > > discussed in another thread > >>> > >> >> "[drlvm] > >>> > >> >> > > >> >> [launcher] Executable hangs". > >>> > >> >> > > >> >> > > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> To get a better idea what's > >>> > >> in the > >>> > >> >> > > >> >> patch.diff, I printed it out. > >>> > >> >> > > >> >> > > > > > > > > > > > >> Its 120+ > >>> > >> >> > > >> >> > > > > > > > > > > > >> pages. Quite a big patch! > >>> > >> >> Most of it looks > >>> > >> >> > > >> >> like straight forward > >>> > >> >> > > >> >> > > > > > > > > > > > >> JNI > >>> > >> >> > > >> >> > > > > > > > > > > > >> interface glue. There are > >>> some > >>> > >> >> tricky > >>> > >> >> > > >> >> parts. I would like to > >>> > >> >> > > >> >> > > > > > > > > > > > >> know the > >>> > >> >> > > >> >> > > > > > > > > > > > >> design review process for > >>> these > >>> > >> >> parts. Using > >>> > >> >> > > >> >> grep, I found 20 > >>> > >> >> > > >> >> > > > > > > > > > > > >> locations > >>> > >> >> > > >> >> > > > > > > > > > > > >> where ...suspend_enable... > >>> and > >>> > >> >> > > >> >> ...suspend_disable... have been > >>> > >> >> > > >> >> > > > > > > > > > > > >> added. And > >>> > >> >> > > >> >> > > > > > > > > > > > >> 25 locations where enable/ > >>> > >> >> disable have been > >>> > >> >> > > >> >> removed. Failure in > >>> > >> >> > > >> >> > > > > > > > > > > > >> this logic > >>> > >> >> > > >> >> > > > > > > > > > > > >> can lead to incorrect > >>> reference > >>> > >> >> pointer > >>> > >> >> > > >> >> enumeration. These are > >>> > >> >> > > >> >> > > > > > > > > > > > >> probably the > >>> > >> >> > > >> >> > > > > > > > > > > > >> hardest bugs to find. Please > >>> > >> >> tell us who has > >>> > >> >> > > >> >> looked at this code > >>> > >> >> > > >> >> > > > > > > > > > > > >> in depth. > >>> > >> >> > > >> >> > > > > > > > > > > > > Only me and you :-) Honetsly I > >>> > >> >> think it > >>> > >> >> > > >> >> happpens now.... > >>> > >> >> > > >> >> > > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > >> Are there any known design > >>> > >> >> flaws in it? > >>> > >> >> > > >> >> > > > > > > > > > > > > I can think of two possible > >>> > >> >> problems we may > >>> > >> >> > > >> >> want to discuss. > >>> > >> >> > > >> >> > > > > > > > > > > > > 1) Should native threads have > >>> > >> >> "daemon" status > >>> > >> >> > > >> >> or its completely java > >>> > >> >> > > >> >> > > > > > > > > > > > > notion? This is TM related > >>> > >> thing. > >>> > >> >> > > >> >> > > > > > > > > > > > > 2) Should we attach thread > >>> to VM > >>> > >> >> before > >>> > >> >> > > >> >> attaching it to TM by calling > >>> > >> >> > > >> >> > > > > > > > > > > > > jthread_atatch OR > >>> jthread_attach > >>> > >> >> should > >>> > >> >> > > >> >> callback VM to attach a thread > >>> > >> >> > > >> >> > > > > > > > > > > > > to it? I didn't change > >>> original > >>> > >> >> design of TM > >>> > >> >> > > >> >> here ...... it implements > >>> > >> >> > > >> >> > > > > > > > > > > > > second choice. > >>> > >> >> > > >> >> > > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> I also notice APIs called > >>> > >> >> > > >> tmn_suspend_enable(), > >>> > >> >> > > >> >> > > > > > > > > > > > >> hythread_suspend_enable() > >>> > >> >> > > >> >> > > > > > > > > > > > >> -- are these simply different > >>> > >> >> names for the > >>> > >> >> > > >> >> same binary > >>> > >> >> > > >> >> > > > > > > > > > > > >> executible. Or > >>> > >> >> > > >> >> > > > > > > > > > > > >> different binaries that do > >>> the > >>> > >> >> same thing?? > >>> > >> >> > > >> >> > > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > > No, this is not just different > >>> > >> >> names. > >>> > >> >> > > >> >> tm_suspend_enable asserts that > >>> > >> >> > > >> >> > > > > > > > > > > > > thread is in disabled state > >>> > >> >> before calling > >>> > >> >> > > >> >> hythread_suspend_enable (in > >>> > >> >> > > >> >> > > > > > > > > > > > > debug mode only). > >>> > >> >> > > >> >> > > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > > Thanks > >>> > >> >> > > >> >> > > > > > > > > > > > > Evgueni > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> > >>> > >> >> > > >> >> > > > > > > > > > > > >> -- > >>> > >> >> > > >> >> > > > > > > > > > > > >> > Weldon Washburn > >>> > >> >> > > >> >> > > > > > > > > > > > >> > 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 > >>> > >> >> > > >> >> > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > >>> > >> >> > > >> >> > > > > > > > > > > > >>> > >> >> > > >> >> > >>> > >> >> > >>> > >> > >>> --------------------------------------------------------------------- > >>> > >> >> > > >> >> > > > > > > > > > > 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 > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > > >>> > >> >> > > >> >> > > -- > >>> > >> >> > > >> >> > > 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 > >>> > >> >> > > >> >> > >>> > >> >> > > >> >> > >>> > >> >> > > >> > > >>> > >> >> > > >> > > >>> > >> >> > > >> > >>> > >> >> > > >> > >>> > >> >> > >>> > >> > >>> --------------------------------------------------------------------- > >>> > >> >> > > >> 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 > >>> > >> > > >>> > >> > >>> > >> > >>> > >> > >>> --------------------------------------------------------------------- > >>> > >> 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 > >> > > > > --------------------------------------------------------------------- > > 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