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. <[EMAIL PROTECTED]> wrote:
>> woo hoo! here we go...
>>
>>
>> Andrey Chernyshev wrote:
>> > Hi Evgueni,
>> >
>> > On 10/4/06, Evgueni Brevnov <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> wrote:
>> >> > > On 10/3/06, Evgueni Brevnov <[EMAIL PROTECTED]> wrote:
>> >> > > > On 10/3/06, Andrey Chernyshev <[EMAIL PROTECTED]> wrote:
>> >> > > > > On 10/2/06, Evgueni Brevnov <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
>> wrote:
>> >> > > > > > > On 9/29/06, Andrey Chernyshev <[EMAIL PROTECTED]>
>> >> wrote:
>> >> > > > > > > > On 9/29/06, Evgueni Brevnov <[EMAIL PROTECTED]>
>> >> wrote:
>> >> > > > > > > > > Artem,
>> >> > > > > > > > >
>> >> > > > > > > > > Thank you for your feedback.... find my inlined.....
>> >> > > > > > > > >
>> >> > > > > > > > > On 9/29/06, Artem Aliev <[EMAIL PROTECTED]> 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
>> >> <[EMAIL PROTECTED]> 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. <[EMAIL PROTECTED]>
>> >> wrote:
>> >> > > > > > > > > > > > So where are we here?
>> >> > > > > > > > > > > >
>> >> > > > > > > > > > > > On Sep 28, 2006, at 12:41 AM, Evgueni Brevnov
>> >> wrote:
>> >> > > > > > > > > > > >
>> >> > > > > > > > > > > > > On 9/28/06, Weldon Washburn
>> >> <[EMAIL PROTECTED]> wrote:
>> >> > > > > > > > > > > > >> On 9/26/06, Evgueni Brevnov
>> >> <[EMAIL PROTECTED]> wrote:
>> >> > > > > > > > > > > > >> >
>> >> > > > > > > > > > > > >> > On 9/27/06, Andrey Chernyshev
>> >> <[EMAIL PROTECTED]> 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:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > > > >
>> >> > > > > > > > > > > >
>> >> > > > > > > > > > > >
>> >> > > > > > > > > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > > > > > > > > Terms of use :
>> >> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > > > > > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > > >
>> >> > > > > > > > > > > >
>> >> > > > > > > > > > >
>> >> > > > > > > > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > > > > > > > Terms of use :
>> >> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > > > > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > >
>> >> > > > > > > > > > >
>> >> > > > > > > > > >
>> >> > > > > > > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > > > > > > Terms of use :
>> >> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > > > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > >
>> >> > > > > > > > > >
>> >> > > > > > > > >
>> >> > > > > > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > > > > > Terms of use :
>> >> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > >
>> >> > > > > > > > >
>> >> > > > > > > >
>> >> > > > > > > >
>> >> > > > > > > > --
>> >> > > > > > > > Andrey Chernyshev
>> >> > > > > > > > Intel Middleware Products Division
>> >> > > > > > > >
>> >> > > > > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > > > > Terms of use :
>> >> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > > >
>> >> > > > > > > >
>> >> > > > > > >
>> >> > > > > >
>> >> > > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > > Terms of use :
>> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > >
>> >> > > > > >
>> >> > > > >
>> >> > > > >
>> >> > > > > --
>> >> > > > > Andrey Chernyshev
>> >> > > > > Intel Middleware Products Division
>> >> > > > >
>> >> > > > >
>> >> ---------------------------------------------------------------------
>> >> > > > > Terms of use :
>> http://incubator.apache.org/harmony/mailing.html
>> >> > > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > >
>> >> > > > >
>> >> > > >
>> >> > > >
>> >> ---------------------------------------------------------------------
>> >> > > > Terms of use : http://incubator.apache.org/harmony/mailing.html
>> >> > > > To unsubscribe, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > > >
>> >> > > >
>> >> > >
>> >> > >
>> >> > > --
>> >> > > Andrey Chernyshev
>> >> > > Intel Middleware Products Division
>> >> > >
>> >> > >
>> ---------------------------------------------------------------------
>> >> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
>> >> > > To unsubscribe, e-mail:
>> [EMAIL PROTECTED]
>> >> > > For additional commands, e-mail:
>> >> [EMAIL PROTECTED]
>> >> > >
>> >> > >
>> >> >
>> >>
>> >> ---------------------------------------------------------------------
>> >> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> >> To unsubscribe, e-mail: [EMAIL PROTECTED]
>> >> For additional commands, e-mail: [EMAIL PROTECTED]
>> >>
>> >>
>> >
>> >
>>
>> ---------------------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: [EMAIL PROTECTED]
>> For additional commands, e-mail: [EMAIL PROTECTED]
>>
>>
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
