> On Jan. 25, 2013, 9:48 a.m., Nathan Binkert wrote: > > src/python/m5/stats/display.py, line 269 > > <http://reviews.gem5.org/r/1643/diff/1/?file=33218#file33218line269> > > > > I don't think that you're improving anything here at all. Are you sure > > you understand __getattr__?
This is not trying to avoid the __getattr__ call. Instead, it is trying to see if the total of the evaluated formula has been passed in from C++ to avoid calculating it in python. This results in a significant speedup. However, you are correct and I have changed the code accordingly. > On Jan. 25, 2013, 9:48 a.m., Nathan Binkert wrote: > > src/python/m5/stats/info.py, line 65 > > <http://reviews.gem5.org/r/1643/diff/1/?file=33219#file33219line65> > > > > If the asserts really aren't needed, it's fine to remove them, but an > > alternative to removing asserts, is to bytecode compile python optimized in > > m5.fast? Removing these asserts increased the stats dumping speed by a significant amount. This is because the scalar() and vector() functions are called extremely often. > On Jan. 25, 2013, 9:48 a.m., Nathan Binkert wrote: > > src/python/m5/stats/info.py, line 580 > > <http://reviews.gem5.org/r/1643/diff/1/?file=33219#file33219line580> > > > > Poor use of __getattr__ I hate to point out that you are the original author of this line. :) That said, how do you propose we change it? > On Jan. 25, 2013, 9:48 a.m., Nathan Binkert wrote: > > src/python/m5/stats/__init__.py, line 353 > > <http://reviews.gem5.org/r/1643/diff/1/?file=33217#file33217line353> > > > > seems like an unrelated fix. Removed. Somehow it crept into one patch and got removed in the other. > On Jan. 25, 2013, 9:48 a.m., Nathan Binkert wrote: > > src/python/m5/stats/display.py, line 174 > > <http://reviews.gem5.org/r/1643/diff/1/?file=33218#file33218line174> > > > > I'm probably missing context, but this seems superfluous. You do the > > same thing below. > > Removed this one. > On Jan. 25, 2013, 9:48 a.m., Nathan Binkert wrote: > > src/python/m5/stats/display.py, line 184 > > <http://reviews.gem5.org/r/1643/diff/1/?file=33218#file33218line184> > > > > Imho, it is very bad form to catch all exceptions here. You will for > > example, trap a KeyboardInterrupt exception which seems like a bad idea. > > Catch the actual exception(s) that you're worried about. To catch more is > > a bug. I have removed the exception, and try and be a bit more intelligent about it. - Sascha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1643/#review3908 ----------------------------------------------------------- On Jan. 29, 2013, 2:47 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1643/ > ----------------------------------------------------------- > > (Updated Jan. 29, 2013, 2:47 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9520:b27555e9cf3d > --------------------------- > stats: Optimise the text-based stats output to improve speed > > The text-based stats output is slow than the original C++ output. This patch > addresses some of those issues by removing: > > Asserts that are not required > stats.ValueProxy, as it is no longer required > Unnecessary calls to __getattr__ and __hasattr__ > > It also changes the formula calculation to only calculate the values and > totals > of formulas if they have not been passed in from the C++. This allows other > formulas to be added, whilst ensuring that the stats output is sufficiently > fast. > > Testing has shown a 6x - 8x increase in stats output speed. > > > Diffs > ----- > > src/python/m5/stats/__init__.py 5532a1642108 > src/python/m5/stats/display.py PRE-CREATION > src/python/m5/stats/info.py PRE-CREATION > > Diff: http://reviews.gem5.org/r/1643/diff/ > > > Testing > ------- > > Ensure that the output files are identical > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
