Aleksey Shipilev wrote: > Want non-committer opinion? :) Of course!
> 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. +1 > BTW, would Chunrong have to re-run the tests from scratch after fixing > HARMONY-6013? Not if the change is local enough that Buqi (or whoever writes it) is confident that targeted testing is going to be enough for this one patch. If it is a design/architectural change then yes we would restart the testing. Regards, Tim > 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 >
