harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.com>
Subject Re: [drlvm] HARMONY-1582 - invocation API for DRLVM CHECKPOINT
Date Thu, 05 Oct 2006 14:55:17 GMT
woo hoo!  here we go...


Andrey Chernyshev wrote:
> Hi Evgueni,
> 
> On 10/4/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> 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 <evgueni.brevnov@gmail.com> 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 <a.y.chernyshev@gmail.com> wrote:
>> > > On 10/3/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
>> > > > On 10/3/06, Andrey Chernyshev <a.y.chernyshev@gmail.com> wrote:
>> > > > > On 10/2/06, Evgueni Brevnov <evgueni.brevnov@gmail.com>
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 <evgueni.brevnov@gmail.com>
wrote:
>> > > > > > > On 9/29/06, Andrey Chernyshev <a.y.chernyshev@gmail.com>

>> wrote:
>> > > > > > > > On 9/29/06, Evgueni Brevnov <evgueni.brevnov@gmail.com>

>> wrote:
>> > > > > > > > > Artem,
>> > > > > > > > >
>> > > > > > > > > Thank you for your feedback.... find my inlined.....
>> > > > > > > > >
>> > > > > > > > > On 9/29/06, Artem Aliev <artem.aliev@gmail.com>
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 
>> <evgueni.brevnov@gmail.com> 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.
<geir@pobox.com> 
>> wrote:
>> > > > > > > > > > > > So where are we here?
>> > > > > > > > > > > >
>> > > > > > > > > > > > On Sep 28, 2006, at 12:41
AM, Evgueni Brevnov 
>> wrote:
>> > > > > > > > > > > >
>> > > > > > > > > > > > > On 9/28/06, Weldon Washburn

>> <weldonwjw@gmail.com> wrote:
>> > > > > > > > > > > > >> On 9/26/06, Evgueni
Brevnov 
>> <evgueni.brevnov@gmail.com> wrote:
>> > > > > > > > > > > > >> >
>> > > > > > > > > > > > >> > On 9/27/06,
Andrey Chernyshev 
>> <a.y.chernyshev@gmail.com> 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


Mime
View raw message