Want non-committer opinion? :) Though I'm the performance guy and have a clue on the cost of adding additional check to monitor_enter, I'm voting for applying null checking code into the stub, then looking into JIT regression. If Buqi will find the clean way to teach JIT to do the right things before M8 is released, then just enroll Buqi's patch. But whatever way it is, this is severe bug and it shouldn't occur on milestone build. Correctness is more important here.
BTW, would Chunrong have to re-run the tests from scratch after fixing HARMONY-6013? Thanks, Aleksey. On Wed, Nov 12, 2008 at 11:31 AM, chunrong lai <[EMAIL PROTECTED]> wrote: > hi, Tim: > The severe uncaught NullPointerException comes with the stub of > vm_rt_monitor_enter. Originally we have two null checkings with that. The > first one is just before the stub and is controled by JIT. The second is > inside the stub. So we had not met the reported exception. > A commit in May (r659128) removeed the null checking (and exception > throwing) inside the stub when doing some optimizations (for HARMONY-5714). > Now we find that the null checking before the stub will also be removed > after some aggressive optimizations. I talked with buqi and he thought that > JIT should never remove such null checking and he is also trying to prepare > a patch to fix this issue. > I think we can have a fast and safe fix for this uncaught exception by > simply adding the null checking code bak to the stub. In this way M8 will > run just like M6, M5 etc. Are there other committers supporting this? We can > also wait for Buqi's (real) fix in JIT to remove the uncaught exception in > M8. > > On Wed, Nov 12, 2008 at 12:00 AM, Tim Ellison <[EMAIL PROTECTED]> wrote: > >> chunrong lai wrote: >> > I can reproduce the error. >> > Zhiguo mentioned that we need to reproduce the error with -Xem:opt or >> > -Xem:server. >> >> Do we know what the fix is for this? >> >> Just wondering if this is a candidate for inclusion in M8, since it was >> a regression since M6. If the patch would not invalidate the results of >> our long running tests then I would like us to consider it since the >> crash is severe. >> >> Regards, >> Tim >> >> > On Tue, Nov 11, 2008 at 9:49 PM, Tim Ellison <[EMAIL PROTECTED]> >> wrote: >> > >> >> Got it -- thanks. >> >> >> >> Can anyone else reproduce HARMONY-6013 [1] ? It works for me with a >> >> simple test, but if others see a failure that would be a pretty bad >> >> regression... >> >> >> >> [1] https://issues.apache.org/jira/browse/HARMONY-6013 >> >> >> >> Regards, >> >> Tim >> >> >> >> chunrong lai wrote: >> >>> Thanks. I am uploading the snapshots. >> >>> >> >>> On Tue, Nov 11, 2008 at 4:53 PM, Sian January < >> >> [EMAIL PROTECTED]>wrote: >> >>>> Chunrong, are you able to make these available? >> >>>> >> >>>> 2008/11/10 Tim Ellison <[EMAIL PROTECTED]>: >> >>>> > Sian January wrote: >> >>>>>> The testing cycle for r710036 [1] has been completed and the test >> >>>>>> results have been discussed [2]. >> >>>>>> >> >>>>>> Two bug fixes [3,4] have been committed since r710036 that we want >> to >> >>>>>> include in M8. r711744 has not been through the extended testing >> >>>>>> cycle, but has been through the standard integrity testing cycle >> with >> >>>>>> the same results as r710036.[5] >> >>>>> Where are the r711744 builds? I don't see them on the snapshots >> >>>>> download page. >> >>>>> >> >>>>> We have to vote on the actual archive bundle, not just a SVN tag. >> >>>>> I'd like to take a look and do a final sanity check. >> >>>>> >> >>>>> Regards, >> >>>>> Tim >> >>>>> >> >>>>>> As usual there are some long-standing issues and a few new ones, but >> >>>>>> nothing that has been considederd critical so far. Overall the pass >> >>>>>> rate is better than M7. >> >>>>>> >> >>>>>> If anyone thinks that a particular issue is a blocker for M8 please >> >> say >> >>>> so here. >> >>>>>> Otherwise, shall we declare r711744 as M8 and unfreeze the code >> base? >> >>>>>> >> >>>>>> [1] >> http://people.apache.org/~chunrong/snapshots/r710036/index.html >> >>>>>> >> >>>>>> [2] http://markmail.org/message/l72lba7xehacqyku >> >>>>>> >> >>>>>> [3] http://markmail.org/message/6fxgpa2azv27zsol >> >>>>>> >> >>>>>> [4] http://markmail.org/message/ljqwytbegtsfou2g >> >>>>>> >> >>>>>> [5] http://people.apache.org/~chunrong/harmony-integrity/ >> >>>>>> >> >>>> >> >>>> -- >> >>>> Unless stated otherwise above: >> >>>> IBM United Kingdom Limited - Registered in England and Wales with >> number >> >>>> 741598. >> >>>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 >> >> 3AU >> > >> >
