> On July 6, 2015, 10:13 p.m., Andreas Hansson wrote: > > I'm not too fond of how this is done, since it essentially creates a base > > class that only adds baggage to the classic memory system. Is it not > > possible to align the two rather? Also, besides the bloat, I am also > > worried about performance. Have you measured the impact on the overall > > regression? > > Nilay Vaish wrote: > You are so predictable. I expected those two comments from > you: that I am adding stuff to classic and that I might hurt performance. > > I agree with you on both the counts. I personally see this > patch a temporary arrangement (that might remain around for long). > > I am trying to improve ruby' speed of simulation. To do that > I need to do away with things that ruby is doing even though those > things are not required. The first thing I am trying to tackle is to > do away with the conversion from a packet to a ruby request. This would > mean that we would need to insert packets into message buffers. > To achieve that end, I need to add this BasePacket class, with both > Packet and Message deriving from it. > > I would like to do away with MessageBuffer class today itself. But > Port and MessageBuffer behave differently. Therefore, I don't think > people would > agree to doing away with the MessageBuffer class that easily. > > Andreas, I think simulation performance would not be impacted since not > much > code is being added (a few instructions at time of construction of a > packet). > I doubt those few instructions would hurt performance. Just for the sake > of argument, assume for a moment that with this patch applied, there is a > 10% increase in simulation time for the classic memory system. I request > you and others who use classic memory system to please bear this loss in > performance for an interim period. As and when we are able to do away > with the MessageBuffer, the BasePacket class would most likely be dropped. > Or redesigned so as to share the common aspects of Packet and Message > classes. > > Andreas Hansson wrote: > Predictable you say. I can live with that :-) > > I'd normally draw the line at ~2%. 10% is _very_ painful (let's quantify > the actual impact). > > Just a thought, can you not simply make the Ruby Message inherit from > Packet?
I will quantify the actual impact on both classic and Ruby, once I get to a state where I think people on both sides would be willing to accept the changes. Message class can inherit from Packet, but that would mean a lot of stuff (much more than what BasePacket adds to Packet) getting added to Message. I initially wanted to drop Message class and use Packet class throughout Ruby. But that would loss of some flexibility that Ruby has wrt messages. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2928/#review6723 ----------------------------------------------------------- On June 29, 2015, 6:19 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2928/ > ----------------------------------------------------------- > > (Updated June 29, 2015, 6:19 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10895:1e201402a79c > --------------------------- > mem: add a base packet class > > This patch adds BasePacket class from which both Packet and Message classes > are > now being derived. This class provides the API which will allows us to push > packets into the ruby memory system directly, without any conversion. In due > course, patches will be posted that do so. These patches would essentially do > away with RubyRequest and remove code from the Sequencer that allocates > SequencerRequest and RubyRequest objects for each packet received from the > processing core. > > > Diffs > ----- > > src/mem/ruby/slicc_interface/Message.hh 73d4798871a5 > src/mem/ruby/slicc_interface/RubyRequest.hh 73d4798871a5 > src/mem/ruby/structures/WireBuffer.cc 73d4798871a5 > src/mem/slicc/symbols/Type.py 73d4798871a5 > src/mem/base_packet.hh PRE-CREATION > src/mem/packet.hh 73d4798871a5 > src/mem/packet.cc 73d4798871a5 > src/mem/ruby/network/MessageBuffer.hh 73d4798871a5 > src/mem/ruby/network/MessageBuffer.cc 73d4798871a5 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 73d4798871a5 > src/mem/ruby/network/garnet/fixed-pipeline/RoutingUnit_d.cc 73d4798871a5 > src/mem/ruby/network/garnet/fixed-pipeline/flit_d.cc 73d4798871a5 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 73d4798871a5 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 73d4798871a5 > src/mem/ruby/network/garnet/flexible-pipeline/flit.cc 73d4798871a5 > src/mem/ruby/network/simple/PerfectSwitch.cc 73d4798871a5 > src/mem/ruby/network/simple/Throttle.cc 73d4798871a5 > > Diff: http://reviews.gem5.org/r/2928/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
