> On 2010-06-07 20:11:13, Nilay Vaish wrote: > > 1. Should we use resize() or should we use reserve() for setting the > > capacity of a vector? Especially during initialization, reserve() might be > > faster since it does not initialize the allocated memory where as resize() > > carries out some initilization of the newly allocated entries. There are > > multiple instances were the vector is first resized and then the entires > > are initialized to some value, thus redoing the work done by resize(). > > There are instances where we may not need the initialization being carried > > out. In those cases we can shift to reserve() and then use push_back() > > instead of []. > > > > 2. In src/mem/gems_common/Map.hh :: keys(), values() -- why not keep using > > the for loop on the iterator? This is probably inconsequential since Nate's > > later patch for Map does away with this file. > > > > 3. I think mem/ruby/common/DataBlock.hh does not need to include <vector>. > > vector is used neither in DataBlock.hh nor in DataBlock.cc. > > > > 4. No need to call clear() in src/mem/ruby/common/NetDest.cc :: > > getAllDest(). In constructors for several classes, clear() function is > > being called on the vector data members of the class. Seems need less to me > > unless an object of the class would be reinitialized. > > > > 5. Do we need to do resize() in network/simple/Switch.cc::Switch()? > > > > 6. In profiler/CacheProfiler.cc, should we move m_requestTypeVec_ptr to > > vector from vector*? > > Nilay Vaish wrote: > Ignore the ship it tag above. I was testing what happens if I check the > box provided. Is it a vote in favor of the patch? > > Nathan Binkert wrote: > 1) It is generally a better practice to try to make changes more self > contained. Switching from resize() to reserve() would in my opinion be doing > two different things in the same patch. I'm fine with you doing this in a > separate patch. > 2) I guess I could have used reserve and push_back. It's not worth > fixing though since as you say, I delete the file anyway. > 3) Fixed > 4) This is an unrelated change. Should be a different changeset. > 5) No. Fixed. > 6) Yes. Done > > Ship It means that you're happy.
This patch also looks set for shipping. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/22/#review38 ----------------------------------------------------------- On 2010-06-02 15:56:35, Nathan Binkert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/22/ > ----------------------------------------------------------- > > (Updated 2010-06-02 15:56:35) > > > Review request for Ruby Reviewers. > > > Summary > ------- > > ruby: get rid of Vector and use STL > add a couple of helper functions to base for deleteing all pointers in > a container and outputting containers to a stream > > > Diffs > ----- > > src/base/stl_helpers.hh PRE-CREATION > src/cpu/rubytest/CheckTable.hh be2acdfb8bdc > src/cpu/rubytest/CheckTable.cc be2acdfb8bdc > src/cpu/rubytest/RubyTester.hh be2acdfb8bdc > src/cpu/rubytest/RubyTester.cc be2acdfb8bdc > src/mem/gems_common/Map.hh be2acdfb8bdc > src/mem/gems_common/PrioHeap.hh be2acdfb8bdc > src/mem/gems_common/Vector.hh be2acdfb8bdc > src/mem/ruby/buffers/MessageBuffer.hh be2acdfb8bdc > src/mem/ruby/common/DataBlock.hh be2acdfb8bdc > src/mem/ruby/common/Histogram.hh be2acdfb8bdc > src/mem/ruby/common/Histogram.cc be2acdfb8bdc > src/mem/ruby/common/NetDest.hh be2acdfb8bdc > src/mem/ruby/common/NetDest.cc be2acdfb8bdc > src/mem/ruby/common/SubBlock.hh be2acdfb8bdc > src/mem/ruby/common/SubBlock.cc be2acdfb8bdc > src/mem/ruby/eventqueue/RubyEventQueue.hh be2acdfb8bdc > src/mem/ruby/filters/BlockBloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/BlockBloomFilter.cc be2acdfb8bdc > src/mem/ruby/filters/BulkBloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/BulkBloomFilter.cc be2acdfb8bdc > src/mem/ruby/filters/H3BloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/H3BloomFilter.cc be2acdfb8bdc > src/mem/ruby/filters/LSB_CountingBloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/LSB_CountingBloomFilter.cc be2acdfb8bdc > src/mem/ruby/filters/MultiBitSelBloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/MultiBitSelBloomFilter.cc be2acdfb8bdc > src/mem/ruby/filters/MultiGrainBloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/MultiGrainBloomFilter.cc be2acdfb8bdc > src/mem/ruby/filters/NonCountingBloomFilter.hh be2acdfb8bdc > src/mem/ruby/filters/NonCountingBloomFilter.cc be2acdfb8bdc > src/mem/ruby/network/Network.hh be2acdfb8bdc > src/mem/ruby/network/Network.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.hh > be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/OutputUnit_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/OutputUnit_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/RoutingUnit_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/RoutingUnit_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/SWallocator_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/SWallocator_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/VCallocator_d.hh be2acdfb8bdc > src/mem/ruby/network/garnet/fixed-pipeline/VCallocator_d.cc be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.hh be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.hh > be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/Router.hh be2acdfb8bdc > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc be2acdfb8bdc > src/mem/ruby/network/simple/PerfectSwitch.hh be2acdfb8bdc > src/mem/ruby/network/simple/PerfectSwitch.cc be2acdfb8bdc > src/mem/ruby/network/simple/SimpleNetwork.hh be2acdfb8bdc > src/mem/ruby/network/simple/SimpleNetwork.cc be2acdfb8bdc > src/mem/ruby/network/simple/Switch.hh be2acdfb8bdc > src/mem/ruby/network/simple/Switch.cc be2acdfb8bdc > src/mem/ruby/network/simple/Throttle.hh be2acdfb8bdc > src/mem/ruby/network/simple/Throttle.cc be2acdfb8bdc > src/mem/ruby/network/simple/Topology.hh be2acdfb8bdc > src/mem/ruby/network/simple/Topology.cc be2acdfb8bdc > src/mem/ruby/profiler/AddressProfiler.cc be2acdfb8bdc > src/mem/ruby/profiler/CacheProfiler.hh be2acdfb8bdc > src/mem/ruby/profiler/CacheProfiler.cc be2acdfb8bdc > src/mem/ruby/profiler/MemCntrlProfiler.hh be2acdfb8bdc > src/mem/ruby/profiler/MemCntrlProfiler.cc be2acdfb8bdc > src/mem/ruby/profiler/Profiler.hh be2acdfb8bdc > src/mem/ruby/profiler/Profiler.cc be2acdfb8bdc > src/mem/ruby/system/CacheMemory.hh be2acdfb8bdc > src/mem/ruby/system/CacheMemory.cc be2acdfb8bdc > src/mem/ruby/system/MemoryVector.hh be2acdfb8bdc > src/mem/ruby/system/Sequencer.cc be2acdfb8bdc > src/mem/ruby/system/System.hh be2acdfb8bdc > src/mem/ruby/system/System.cc be2acdfb8bdc > src/mem/ruby/system/TimerTable.cc be2acdfb8bdc > src/mem/ruby/tester/DeterministicDriver.hh be2acdfb8bdc > src/mem/ruby/tester/DeterministicDriver.cc be2acdfb8bdc > src/mem/ruby/tester/RaceyDriver.hh be2acdfb8bdc > src/mem/ruby/tester/RaceyDriver.cc be2acdfb8bdc > src/mem/slicc/symbols/StateMachine.py be2acdfb8bdc > > Diff: http://reviews.m5sim.org/r/22/diff > > > Testing > ------- > > All regressions pass > > > Thanks, > > Nathan > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
