harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Artem Aliev" <artem.al...@gmail.com>
Subject Re: [drlvm] HARMONY-1582 - invocation API for DRLVM CHECKPOINT
Date Thu, 05 Oct 2006 14:19:15 GMT
+1 from me

Artem


On 10/5/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
> can we get a green light from Andrey and any others in the conversation?
>
> geir
>
>
> Evgueni Brevnov wrote:
> > Hi,
> >
> > I'm working on restoring cunit tests. I already made a good progress
> > in that direction. So I expect to have it done soon....
> >
> > Evgueni
> >
> > On 10/4/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
> >> On 10/4/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
> >> > I assume you intend that only the latest patch is applied?
> >> Yes. invocation_api.5.patch only.
> >> > (And I assume that it would apply cleanly to SVN HEAD)
> >> I believe so.
> >>
> >> Evgueni
> >> >
> >> > geir
> >> >
> >> > 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
> >> > > 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
> >> >
> >> >
> >>
> >
> > ---------------------------------------------------------------------
> > 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