> 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.)
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. - Nathan ----------------------------------------------------------- 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 gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
