I just double checked, and the gem5 repository being used for regressions (as checked out on the machine) doesn't have all the stats fixes I checked in a couple days ago. I'd really like to make it use git instead of hg, but I don't have write access to the necessary files.
Gabe On Wed, Apr 5, 2017 at 11:36 AM, Andreas Sandberg <[email protected]> wrote: > That's caused by a setting in gerrit. I think we just forgot to turn on > copyAllScoresOnTrivialRebase for the maintainer label. > > > //Andreas > > ________________________________ > From: gem5-dev <[email protected]> on behalf of Gabe Black < > [email protected]> > Sent: Wednesday, April 5, 2017 7:25:36 PM > To: gem5 Developer List > Subject: Re: [gem5-dev] nightly regressions > > I had to rebase my CLs, and that seems to have dropped the maintainer +1. > Could you please reapply? Hopefully that's not something we'll have to do > for each patch, although I think before I had to rebase for each patch in a > series. Do you know if there's some setting that's making that necessary? > > Gabe > > On Wed, Apr 5, 2017 at 11:09 AM, Gabe Black <[email protected]> wrote: > > > No, I think you reviewed the only one which doesn't just update the > stats. > > It would be fine to just mark the other ones as good to go. > > > > Gabe > > > > On Wed, Apr 5, 2017 at 10:50 AM, Jason Lowe-Power <[email protected]> > > wrote: > > > >> Hi Gabe, > >> > >> Thanks for going through and updating the stat files for all of the > recent > >> changes. The people who used to volunteer to do that haven't had time > >> lately. > >> > >> Is there any reason I shouldn't just check off on all of the stats > >> changes? > >> Is there anything in the changesets to review? > >> > >> Cheers, > >> Jason > >> > >> On Tue, Apr 4, 2017 at 11:29 AM Steve Reinhardt <[email protected]> > wrote: > >> > >> > Here's the changeset Andreas was referring to: > >> > > >> > http://repo.gem5.org/gem5?cmd=changeset;node=b2720503a978 > >> > > >> > > >> > On Tue, Apr 4, 2017 at 9:20 AM, Gabe Black <[email protected]> > >> wrote: > >> > > >> > > I can't add reviewers due to a known issue, but here is the stack of > >> CLs > >> > > which fix the regressions: > >> > > > >> > > https://gem5-review.googlesource.com/#/c/2641/ > >> > > > >> > > Gabe > >> > > > >> > > On Tue, Apr 4, 2017 at 3:35 AM, Gabe Black <[email protected]> > >> wrote: > >> > > > >> > > > Oh, also, the problem with EIO I think is not because there are > EIO > >> > > > regressions, it's that the EIO code is being added in with EXTRAS > >> and > >> > > > breaking the build for certain targets. Excluding it would be > fairly > >> > > > trivial if I had write permission for the regression script on > >> zizzer. > >> > > > > >> > > > Gabe > >> > > > > >> > > > On Tue, Apr 4, 2017 at 3:32 AM, Gabe Black <[email protected]> > >> > wrote: > >> > > > > >> > > >> Yeah, there are a lot of problems with the current system. It has > >> > three > >> > > >> real functions: > >> > > >> > >> > > >> 1. Making sure things run from end to end without crashing. > >> > > >> 2. Checking for inadvertent changes to the stats. > >> > > >> 3. Checking for non-determinstic behavior. > >> > > >> > >> > > >> Unfortunately it takes a really long time to run, doesn't > >> distinguish > >> > > >> between trivial and non-trivial divergence in behavior, is really > >> hard > >> > > to > >> > > >> merge, doesn't tell you what's not working if something is > broken, > >> has > >> > > >> dependencies with major complications, isn't very useful when > >> people > >> > > >> diverge from upstream, etc. > >> > > >> > >> > > >> I think there is definitely a role for that sort of test since > >> those > >> > are > >> > > >> meaningful, but then there are lots and lots of potential testing > >> that > >> > > >> we're not doing. We should have the few unit tests that already > >> exist > >> > > all > >> > > >> either pass or fail, not just output something, and then make > them > >> > > >> easy/automatic to run, and we should have more of them. There are > >> vast > >> > > >> swathes of functionality that really should be tested with unit > >> tests > >> > > like: > >> > > >> > >> > > >> 1. The CPU models. > >> > > >> 2. All the instructions for all the ISAs. > >> > > >> 3. All the Device models. > >> > > >> etc. etc. > >> > > >> > >> > > >> The amount of work it would/will take to cover the gap in testing > >> is > >> > > >> enormous, but in my opinion that's the technical debt that has to > >> be > >> > > paid > >> > > >> down to straighten things out. > >> > > >> > >> > > >> Gabe > >> > > >> > >> > > >> On Tue, Apr 4, 2017 at 2:55 AM, Andreas Sandberg < > >> > > >> [email protected]> wrote: > >> > > >> > >> > > >>> On 01/04/2017 12:27, Gabe Black wrote: > >> > > >>> > >> > > >>>> Hi folks. I'm working through the nightly regressions to get > them > >> > to a > >> > > >>>> good > >> > > >>>> point for a rebase of our internal branch of gem5, and I've > >> noticed > >> > a > >> > > >>>> few > >> > > >>>> things: > >> > > >>>> > >> > > >>>> 1. The stats have been changed but not updated a bunch of > times. > >> > I've > >> > > >>>> identified almost all the points this has happened since the > >> > > references > >> > > >>>> were last updated, and have patches which fix them. Some stat > >> > changes > >> > > >>>> are a > >> > > >>>> little fishy, but I'll at least identify the guilty change(s) > so > >> > that > >> > > >>>> their > >> > > >>>> authors can look them over. > >> > > >>>> > >> > > >>> > >> > > >>> This is really hard to get right with the current system of > "push > >> to > >> > > >>> submit". I would really like to avoid including stat updates in > >> > normal > >> > > >>> code submissions. It would make it really hard to automatically > >> > submit > >> > > >>> code (there would be stat conflicts for every single non-trivial > >> > > change) > >> > > >>> and it'd make cherry-picking really annoying. > >> > > >>> > >> > > >>> Ideally, the CI system should compare the stat output after > >> applying > >> > a > >> > > >>> CL to the previous stat update. That way, you can easily spot > the > >> > > >>> difference when submitting new code. > >> > > >>> > >> > > >>> 2. The SPARC FS regression were just plain not running because > its > >> > > >>>> configuration had been broken. I'll have a patch to fix this. > >> > > >>>> 3. The nightly regressions are still checking gem5 out from > >> > mercurial. > >> > > >>>> > >> > > >>> > >> > > >>> We should obviously fix this. However, the repo is kept in sync > >> with > >> > > the > >> > > >>> golden git repo using a cron, so it's not quite as bad as it > >> seems. > >> > > >>> > >> > > >>> 4. The "encumbered" repository has, as far as I can tell, not be > >> > > >>>> converted > >> > > >>>> from mercurial to git. Probably this isn't a problem because > this > >> > code > >> > > >>>> is > >> > > >>>> mostly unchanging and becoming less relevant over time, > >> especially > >> > > since > >> > > >>>> EIO support was removed from the process classes (it was, > >> right?). > >> > > >>>> 5. The EIO code is also broken, because it tries to call > "fatal" > >> > with > >> > > a > >> > > >>>> "(void)" cast in front of it in a ternary operation. Something > >> like > >> > > >>>> "foo ? > >> > > >>>> (void)fatal("a bad thing happened") : (void)fatal("a different > >> bad > >> > > thing > >> > > >>>> happened")". What "fatal" expands to now is apparently not > >> > compatible > >> > > >>>> with > >> > > >>>> this usage. Since I can access the encumbered repository, I can > >> > > attempt > >> > > >>>> to > >> > > >>>> fix this. > >> > > >>>> I can (and in at least in some cases will) fix most of these > >> issues, > >> > > >>>> except > >> > > >>>> maybe 4 if we still want encumbered to exist. Please speak up > if > >> > I've > >> > > >>>> misinterpreted something or am missing some important bit of > >> > context. > >> > > >>>> [1] https://www.mail-archive.com/[email protected]/msg19438. > html > >> > > >>>> > >> > > >>> > >> > > >>> As far as I know, the encumbered/EIO repo has been deprecated. > >> There > >> > > was > >> > > >>> a discussion about deprecating old and broken ISAs a while back > >> [1], > >> > > >>> that resulted in removing EIO as a way to reduce the test > >> overhead. > >> > > >>> Steve sent out an email, which no one replied to, asking for EIO > >> > users > >> > > >>> [2] shortly after the discussion. I think he disabled EIO tests > >> after > >> > > >>> that. > >> > > >>> > >> > > >>> We should probably think about what fixing means in this case. I > >> have > >> > > >>> actively removed stat diffing from tests that I consider to be > >> > > >>> functional (e.g., atomic boot, CPU switching, checkpointing). > >> > > >>> > >> > > >>> I wouldn't mind disabling stat diffing altogether until we have > >> > figured > >> > > >>> out what to do for performance regressions. I'd prefer to see > some > >> > more > >> > > >>> directed performance tests that target specific components > (e.g., > >> > using > >> > > >>> traces) and specific stats. > >> > > >>> > >> > > >>> //Andreas > >> > > >>> > >> > > >>> [2] https://www.mail-archive.com/[email protected]/msg19379. > html > >> > > >>> IMPORTANT NOTICE: The contents of this email and any attachments > >> are > >> > > >>> confidential and may also be privileged. If you are not the > >> intended > >> > > >>> recipient, please notify the sender immediately and do not > >> disclose > >> > the > >> > > >>> contents to any other person, use it for any purpose, or store > or > >> > copy > >> > > the > >> > > >>> information in any medium. Thank you. > >> > > >>> > >> > > >>> _______________________________________________ > >> > > >>> gem5-dev mailing list > >> > > >>> [email protected] > >> > > >>> http://m5sim.org/mailman/listinfo/gem5-dev > >> > > >>> > >> > > >> > >> > > >> > >> > > > > >> > > _______________________________________________ > >> > > gem5-dev mailing list > >> > > [email protected] > >> > > http://m5sim.org/mailman/listinfo/gem5-dev > >> > > > >> > _______________________________________________ > >> > gem5-dev mailing list > >> > [email protected] > >> > http://m5sim.org/mailman/listinfo/gem5-dev > >> _______________________________________________ > >> gem5-dev mailing list > >> [email protected] > >> http://m5sim.org/mailman/listinfo/gem5-dev > >> > > > > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
