> On 2011-05-17 12:13:18, Nathan Binkert wrote: > > src/mem/ruby/profiler/Profiler.cc, line 456 > > <http://reviews.m5sim.org/r/704/diff/1/?file=12599#file12599line456> > > > > How did you pick this magic number? Can we at least make it a constant > > that is defined in a header file with an explanation as to how it was > > chosen? > > Derek Hower wrote: > All magic numbers came from whatever Ruby was doing before the conversion > (typically in the default initialization for Histograms). Making it a > constant sounds fine. > > Korey Sewell wrote: > I can attest to these hardcoded values being the root of many (not all!) > Ruby evils. > > This is the histogram bucket size and the tricky thing is these also get > reset with a clear(200) in the clearStats() function. The clearStats() > functions should get whacked (nate's comments below), but ideally, make the > histogram bucket size a parameter with a default value here. > > Korey Sewell wrote: > The histogram is initialized with size per bin and also number of bins. > > I really don't think this should be a constant (this has caused me some > headache before). > > Instead, a parameter with a default value of 200 sounds about right, no? > Also, where does the bin size get initialized for the histogram stat? > (Another candidate for a default parameter.) > > Nathan Binkert wrote: > Yeah, a parameter does make sense. As for bin size, that's automatic. > Bin sizes start at 1 and automatically double until they can hold all samples.
I agree that makes sense in some instances, but I've found that when you do that, it makes it hard to extract stats that can compare to runs of different systems. For example, when I'm comparing stats across different systems, I want to easily compare how many times a coherency miss fell within the 10-20 cycle range. When you don't have that, you have one run w/a stat range of 10-20 and another with 10-50. That gets messy ... quick. That's why I think it's advantageous to define a bin size and then a saturation point. Maybe called that "saturating histogram" instead of the regular histogram. It was a easy fix for me to do in the Ruby tree, but if that's not something straightforward for gem5, just mark that down as a @todo or limitation of the histograms. - Korey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/704/#review1237 ----------------------------------------------------------- On 2011-05-16 15:06:16, Derek Hower wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/704/ > ----------------------------------------------------------- > > (Updated 2011-05-16 15:06:16) > > > Review request for Default, Nathan Binkert, Korey Sewell, and Brad Beckmann. > > > Summary > ------- > > This patch contains changes to convert Ruby's stat handling to the M5-style > Stat class. The ultimate goal is to remove Profiler entirely, though this > patch only represents a (significant) step towards that goal. Some stats > remain in the Profiler, notably those that do not have an obvious object to > hold them (like Address Profiler) or those that I'm not sure what they do > (e.g., *wCC*). Also, this patch does not contain a Garnet stats conversion > (though the simple network is converted). > > > Diffs > ----- > > src/mem/ruby/network/simple/SimpleNetwork.hh UNKNOWN > src/mem/ruby/network/simple/SimpleNetwork.cc UNKNOWN > src/mem/ruby/network/simple/Throttle.hh UNKNOWN > src/mem/ruby/network/simple/Throttle.cc UNKNOWN > src/mem/ruby/profiler/AddressProfiler.hh UNKNOWN > src/mem/ruby/profiler/AddressProfiler.cc UNKNOWN > src/mem/ruby/profiler/CacheProfiler.hh UNKNOWN > src/mem/ruby/profiler/CacheProfiler.cc UNKNOWN > src/mem/ruby/profiler/MemCntrlProfiler.hh UNKNOWN > src/mem/ruby/profiler/MemCntrlProfiler.cc UNKNOWN > src/mem/ruby/profiler/Profiler.hh UNKNOWN > src/mem/ruby/profiler/Profiler.cc UNKNOWN > src/mem/ruby/slicc_interface/RubySlicc_Profiler_interface.hh UNKNOWN > src/mem/ruby/slicc_interface/RubySlicc_Profiler_interface.cc UNKNOWN > src/mem/ruby/system/CacheMemory.hh UNKNOWN > src/mem/ruby/system/MemoryControl.hh UNKNOWN > src/mem/ruby/system/Sequencer.hh UNKNOWN > src/mem/ruby/system/Sequencer.cc UNKNOWN > src/mem/ruby/system/System.cc UNKNOWN > src/mem/slicc/symbols/StateMachine.py UNKNOWN > > Diff: http://reviews.m5sim.org/r/704/diff > > > Testing > ------- > > > Thanks, > > Derek > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
