----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/21/#review37 -----------------------------------------------------------
Ship it! I haven't looked at this in great detail (and don't plan to), but I'm wondering: I don't know what the purpose of Allocator() was, but does it make sense to replace it with FastAlloc rather than just leaving it to the default allocator? I see where this would be a little more complicated since it involves tracking down the classes that used Allocator and changing their inheritance, but I figured I'd throw it out there. I'm fine with doing this in a separate patch if it's a good idea. - Steve On 2010-06-02 15:56:26, Nathan Binkert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/21/ > ----------------------------------------------------------- > > (Updated 2010-06-02 15:56:26) > > > Review request for Ruby Reviewers. > > > Summary > ------- > > ruby: get rid of RefCnt and Allocator stuff use base/refcnt.hh > > This was somewhat tricky because the RefCnt API was somewhat odd. The > biggest confusion was that the the RefCnt object's constructor that > took a TYPE& cloned the object. I created an explicit virtual clone() > function for things that took advantage of this version of the > constructor. I was conservative and used clone() when I was in doubt > of whether or not it was necessary. I still think that there are > probably too many instances of clone(), but hopefully not too many. > > I converted several instances of const MsgPtr & to a simple MsgPtr. > If the function wants to avoid the overhead of creating another > reference, then it should just use a regular pointer instead of a ref > counting ptr. > > There were a couple of instances where refcounted objects were created > on the stack. This seems pretty dangerous since if you ever > accidentally make a reference to that object with a ref counting > pointer, bad things are bound to happen. > > > Diffs > ----- > > src/mem/gems_common/Allocator.hh 9ea24d102d66 > src/mem/gems_common/RefCnt.hh 9ea24d102d66 > src/mem/gems_common/RefCnt_tester.cc 9ea24d102d66 > src/mem/gems_common/RefCountable.hh 9ea24d102d66 > src/mem/gems_common/SConscript 9ea24d102d66 > src/mem/ruby/buffers/MessageBuffer.hh 9ea24d102d66 > src/mem/ruby/buffers/MessageBuffer.cc 9ea24d102d66 > src/mem/ruby/common/Debug.hh 9ea24d102d66 > src/mem/ruby/common/Debug.cc 9ea24d102d66 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 9ea24d102d66 > src/mem/ruby/network/garnet/fixed-pipeline/RoutingUnit_d.cc 9ea24d102d66 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 9ea24d102d66 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 9ea24d102d66 > src/mem/ruby/network/simple/PerfectSwitch.cc 9ea24d102d66 > src/mem/ruby/network/simple/Throttle.cc 9ea24d102d66 > src/mem/ruby/slicc_interface/Message.hh 9ea24d102d66 > src/mem/ruby/slicc_interface/NetworkMessage.hh 9ea24d102d66 > src/mem/ruby/system/DMASequencer.cc 9ea24d102d66 > src/mem/ruby/system/MemoryControl.cc 9ea24d102d66 > src/mem/ruby/system/Sequencer.cc 9ea24d102d66 > src/mem/slicc/ast/EnqueueStatementAST.py 9ea24d102d66 > src/mem/slicc/symbols/Type.py 9ea24d102d66 > > Diff: http://reviews.m5sim.org/r/21/diff > > > Testing > ------- > > All regressions pass > > > Thanks, > > Nathan > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
