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]
