> On July 24, 2015, 4:48 p.m., Jason Power wrote: > > I'm not sure how I feel about this patch (and the other ones posted today) > > in general, but if we are going to move away from reference counting, then > > this patch needs to include very specific information on which objects are > > responsible for deleting any heap variables. As it stands, it would be very > > hard for someone to modify this code without introducing spurious memory > > bugs. > > Jason Power wrote: > Nilay said: > > The question of responsibility arises when multiple objects may hold > > pointer to a heap allocated object. As per my understanding, this is > not > > the case for any of the protocols and on-chip network implementations. > > So the current owner can choose to delete the message as soon as it > > realizes that the message is no longer required. > > So you're saying that for it is always the receiver's responsibility to > delete the message? Is this the same protocol we use for packets? I just > worry about memory bugs creeping up when someone is modifying a protocol. > That would be incredibly hard to track down! > > Andreas Hansson wrote: > The explicit allocation/deletion in the classic memory system is a tricky > story, and we have looked at using shared_ptr quite a few times. Only doing > it for requests costs us around 10% performance. For packets it's worse. > > The reason the new/delete is not straight forward is that packets have > different lifetimes. Request packets that do not need responses are deleted > when they reach the slave, e.g. a memory. Packets that need need a response > are typically re-used, and the response packet is deleted at the master, e.g. > a device or CPU. The big issue is forwarding of packets, which happens in the > caches. Here it gets rather tricky to track the lifetime of the request > (outliving the copy), and each packet. > > If changing to explicit allocation/deletion brings a lot of performance, > then perhaps it is the right answer. Beware though...the devil is in the > details.
I think ruby's choice of generating new messages at each step is better compared to the classic's decision of reusing packets. That way it is always the receivers's responsibility to delete messages they receive. And I agree with Andreas that shared pointers have a big cost. I see a small improvement of about 2% but that is because other parts of ruby are slow as well (ruby without any of my proposed changes is about 4-5 times slower, overall simulations are about two times slower). - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2989/#review6824 ----------------------------------------------------------- On July 24, 2015, 4:46 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2989/ > ----------------------------------------------------------- > > (Updated July 24, 2015, 4:46 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10952:57fd09945e80 > --------------------------- > ruby: switch to explicit allocation and deletion of messages > Currently messages in ruby are reference counted objects. This is a > performance issue > since c++ reference counting uses atomic instructions for maintaining the > counts. > This patch explicitly allocates and deletes messages. We see a performance > improvement > of about 3-5% on the only ruby fs regression test. > > > Diffs > ----- > > src/mem/ruby/slicc_interface/Message.cc PRE-CREATION > src/mem/ruby/slicc_interface/RubyRequest.hh 9689ead7b479 > src/mem/ruby/slicc_interface/SConscript 9689ead7b479 > src/mem/ruby/structures/WireBuffer.hh 9689ead7b479 > src/mem/ruby/structures/WireBuffer.cc 9689ead7b479 > src/mem/ruby/system/DMASequencer.cc 9689ead7b479 > src/mem/ruby/system/Sequencer.cc 9689ead7b479 > src/mem/slicc/symbols/Type.py 9689ead7b479 > src/mem/protocol/MESI_Two_Level-L1cache.sm 9689ead7b479 > src/mem/protocol/MESI_Two_Level-L2cache.sm 9689ead7b479 > src/mem/protocol/MESI_Two_Level-dir.sm 9689ead7b479 > src/mem/protocol/MESI_Two_Level-dma.sm 9689ead7b479 > src/mem/ruby/network/MessageBuffer.hh 9689ead7b479 > src/mem/ruby/network/MessageBuffer.cc 9689ead7b479 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.hh > 9689ead7b479 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 9689ead7b479 > src/mem/ruby/network/garnet/fixed-pipeline/RoutingUnit_d.cc 9689ead7b479 > src/mem/ruby/network/garnet/fixed-pipeline/flit_d.hh 9689ead7b479 > src/mem/ruby/network/garnet/fixed-pipeline/flit_d.cc 9689ead7b479 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.hh > 9689ead7b479 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 9689ead7b479 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 9689ead7b479 > src/mem/ruby/network/garnet/flexible-pipeline/flit.hh 9689ead7b479 > src/mem/ruby/network/garnet/flexible-pipeline/flit.cc 9689ead7b479 > src/mem/ruby/network/simple/PerfectSwitch.cc 9689ead7b479 > src/mem/ruby/network/simple/Throttle.cc 9689ead7b479 > src/mem/ruby/slicc_interface/AbstractController.cc 9689ead7b479 > src/mem/ruby/slicc_interface/Message.hh 9689ead7b479 > > Diff: http://reviews.gem5.org/r/2989/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
