I mahaged to resolve the problem on Linux.... will update
build.final.patch with build.final.2.patch in a while
On 10/8/06, Evgueni Brevnov <[EMAIL PROTECTED]> wrote:
> Hi,
>
> Oh! Ooh! I did that..... I passed cunit, somke, kernel tests on
> Windows and smoke, kernel tests on Linux. Unfortunately I failed to
> link cunit tests on Linux so far. So I disabled cunit on Linux
until
> the problem is solved. I believe it is acceptable as short term
> solution. I found several problems in cunit tests. I will come
up with
> my findings later (not today).
>
> Use latest patches from HARMONY-1582. They are marked as final.
There
> are three patches. One for build module, one for cunit tests and
one
> for VM itself.
>
> Thanks
> Evgueni
>
>
> On 10/6/06, Geir Magnusson Jr. <[EMAIL PROTECTED]> wrote:
> > Nooooooo! LOL
> >
> > I'm here waiting - This will unblock a whole bunch of things :)
> >
> > Thanks for the effort
> >
> > Evgueni Brevnov wrote:
> > > Geir,
> > >
> > > That's terrible. We have power outage....servers are down. I
can't
> > > send the patches now.... will do it tomorrow
> > >
> > > Evgueni
> > > On 10/5/06, Geir Magnusson Jr. <[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: harmony-dev-
[EMAIL PROTECTED]
> > >> >> For additional commands, e-mail: harmony-dev-
[EMAIL PROTECTED]
> > >> >>
> > >> >>
> > >> >
> > >> >
> > >>
> > >>
---------------------------------------------------------------------
> > >> Terms of use : http://incubator.apache.org/harmony/
mailing.html
> > >> To unsubscribe, e-mail: harmony-dev-
[EMAIL PROTECTED]
> > >> For additional commands, e-mail: harmony-dev-
[EMAIL PROTECTED]
> > >>
> > >>
> > >
> > >
---------------------------------------------------------------------
> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > To unsubscribe, e-mail: harmony-dev-
[EMAIL PROTECTED]
> > > For additional commands, e-mail: harmony-dev-
[EMAIL PROTECTED]
> > >
> >
> >
---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-
[EMAIL PROTECTED]
> > For additional commands, e-mail: harmony-dev-
[EMAIL PROTECTED]
> >
> >
>
