On 9/30/06, Weldon Washburn <[EMAIL PROTECTED]> wrote:
Evgueni,
+1 provided "build, build test" run on Windows, Redhat and Unbuntu.
I looked carefully at all the enable/disable sites. AFAICT, they look
perfect. You did a good job!
Thank you!
Please correct me if I am wrong, the enable/disable sites are in "JNI"
methods where you need to step outside of JNI environment to actually touch
a slot holding a reference ptr. In which case, you need to disable, access
the slot, then re-enable suspension.
Correct.
I agree that supporting multiple JVMs in one addr space is a non-goal for
drlvm at this time. But at the same time it make sense to be as compatible
as possible with portlib. Portlib is really setup to expect multiple JVM in
one addr space environment. The bottom line is we should put in whatever
hooks portlib but only run drlvm in "one vm per addr space" mode.
Sounds reasonable for me.
Evgueni
On 9/29/06, Evgueni Brevnov <[EMAIL PROTECTED]> wrote:
>
> Weldon that's cool if you review the patch. Regarding enable/disable
> switching. I agree here such bugs are quite hard to fix. Actually I
> think it is a good indicator that you found 25 (don't remember
> exactly) enable/disable switching. It was really suspicious if you
> hadn't find any disabled region :-)
>
> I will update the patch as with respect to your recommendations very
> soon....
>
> Evgueni
>
> On 9/29/06, Tim Ellison <[EMAIL PROTECTED]> wrote:
> > IMHO Weldon is making a perfectly reasonable request. If he is willing
> > to look through the patch in detail then waiting a day or two on
> > progressing other items is well worth it.
> >
> > Just my 2c.
> >
> > Tim
> >
> > Weldon Washburn wrote:
> > > 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.
> > >
> > >
> > > Sorry for not being clear. I had asked in another thread for a
> readable
> > > diff. As I said before, hacking around with gc enable/disable without
> > > careful review is a great way to introduce all sorts of hard to
> > > diagnose intermittant threading/gc bugs. The existing "build test"
> does
> > > not
> > > even come close to stressing threading/gc. Its hard to say if this
> patch
> > > really works at this point.
> > >
> > > I request a decent diff and 24 hours for Andrey Cherneyshev and me to
> > > review. I think the following will work:
> > >
> > > "svn diff --diff-cmd diff.exe -x -w -x -B"
> > >
> > >
> > > 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]
> > >>
> > >>
> > >
> > >
> >
> > --
> >
> > Tim Ellison ([EMAIL PROTECTED])
> > IBM Java technology centre, UK.
> >
> > ---------------------------------------------------------------------
> > 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]
>
>
--
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]
