Mailing-List: contact harmony-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: harmony-dev@incubator.apache.org
DomainKey-Status: good
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references;
        b=KkOl4giAfCxv2ZUFMRGJZUbecIh7f+RdMeeQkGxQRMAxyzUgIC2LkPc1bsRQstmekWSmW6hc4ENs0pcC6fs95T632I8HyHJ9C65W5ZtnI4qJubaJpolRm0zkYiLmdNe6xrx2cguaPmd462a/q68knbdIGmay1BlqNe8dqwVeie4=
Message-ID: <e29119980610020528i4f2ab227nc0af76d648238d6e@mail.gmail.com>
Date: Mon, 2 Oct 2006 19:28:49 +0700
From: "Evgueni Brevnov" <evgueni.brevnov@gmail.com>
To: harmony-dev@incubator.apache.org
Subject: Re: [drlvm] HARMONY-1582 - invocation API for DRLVM CHECKPOINT
In-Reply-To: <4dd1f3f00609291553y2f44eeeer3e62913cc410bcd8@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <79F58F0A-803A-41CD-B6FF-6E92124BD8AB@pobox.com>
	 <e29119980609262338n44444c5bvecfc7e04e4320e92@mail.gmail.com>
	 <4dd1f3f00609271304s553b30f4k93bf9fdbe60b425e@mail.gmail.com>
	 <e29119980609272141u7c867093kb2567c192df4e045@mail.gmail.com>
	 <CA6BC2A6-19B1-43CA-A79A-967F4FFD1B38@pobox.com>
	 <e29119980609280619w137d90e0p9193283a9e3e0192@mail.gmail.com>
	 <4dd1f3f00609282141o45d467a3u11518fa21482b7ca@mail.gmail.com>
	 <451CD329.5030305@gmail.com>
	 <e29119980609290130w29222e2aiba47235c0b4488@mail.gmail.com>
	 <4dd1f3f00609291553y2f44eeeer3e62913cc410bcd8@mail.gmail.com>

On 9/30/06, Weldon Washburn <weldonwjw@gmail.com> 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 <evgueni.brevnov@gmail.com> 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 <t.p.ellison@gmail.com> 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 <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.
> > > >
> > > >
> > > > 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. <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
> > > >>
> > > >>
> > > >
> > > >
> > >
> > > --
> > >
> > > Tim Ellison (t.p.ellison@gmail.com)
> > > IBM Java technology centre, UK.
> > >
> > > ---------------------------------------------------------------------
> > > 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
> >
> >
>
>
> --
> 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