> On Aug. 14, 2015, 9:17 p.m., Brad Beckmann wrote: > > Thanks Nilay for sending out this patch to start the discussion. Unlike > > some of the other recent patches you posted, this one has obvious benefits > > and should not adversly impact too much downstream code. Having the > > Consumer class use AutoDelete Events was never meant to be the long-term > > solution. It was simply the easiest way for us to complete the initial > > merger of GEMS and M5. I'm glad to see this improvement. > > > > So can you comment on whether you see this as an eventual set to eliminate > > the Consumer class?
That's the aim. > On Aug. 14, 2015, 9:17 p.m., Brad Beckmann wrote: > > src/mem/ruby/network/MessageBuffer.hh, line 140 > > <http://reviews.gem5.org/r/2987/diff/1/?file=48401#file48401line140> > > > > Please use a longer name than em. This is not a local variable, but > > rather it is now a very important member for MessageBuffer. It deserves a > > more descriptive name and yes, I understand that there may be bad precedent > > in other parts of the code. For a variable as important this the > > EventManger, we should make sure it is easy to search for. OK. > On Aug. 14, 2015, 9:17 p.m., Brad Beckmann wrote: > > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc, line 117 > > <http://reviews.gem5.org/r/2987/diff/1/?file=48403#file48403line117> > > > > Does this make sense to set the event manager but not set the event > > associated with the event manager? I probably do not fully appreciate the > > EventManager design, but I thought it existed just to make sure that a > > single event was only scheduled on a single event queue. It seems like the > > code requires the event to be set as well. > > > > Perhaps this is why you say the patch is incomplete and that Garnet > > will not work. Garnet is still incomplete. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2987/#review6960 ----------------------------------------------------------- On July 24, 2015, 4:45 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2987/ > ----------------------------------------------------------- > > (Updated July 24, 2015, 4:45 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10948:12a8ecefe478 > --------------------------- > ruby: consumer: move away from Consumer class > > Objects in ruby memory system typically inherit from the Consumer class that > provides support for scheduling events. The Consumer class maintains a > std::set > of times at which events have been posted by the Derived class object. > Typically > this causes a lot of overhead. Secondly, the objects schedule events that are > very coarse grained. This patch reduces ruby memory system's reliance on the > Consumer > class. > > The patch changes the objects in Simple Network and the generated code for > Controllers > in a significant way. The generated controllers would now schedule events for > individual input ports and not for the entire controller itself. Similarly, > PerfectSwitch > and Throttle would now schedule events on individual message buffers and not > for the > entire object. This avoids looking at buffers that do not have any message > pending. > > This patch is incomplete in the sense that Garnet would not work if this > patch is committed. > Fixing Garnet would take some time due to its own peculiarities. The patch > is being > put up more from a discussion point of view before I embark on changing > Garnet. > > Overall we see about 15-20% improvement in performance of the only ruby fs > regression > test we have in our suite. > > > Diffs > ----- > > src/mem/ruby/network/simple/PerfectSwitch.cc 9689ead7b479 > src/mem/ruby/network/simple/Throttle.hh 9689ead7b479 > src/mem/ruby/network/simple/Throttle.cc 9689ead7b479 > src/mem/ruby/slicc_interface/AbstractController.hh 9689ead7b479 > src/mem/ruby/slicc_interface/AbstractController.cc 9689ead7b479 > src/mem/ruby/slicc_interface/Message.hh 9689ead7b479 > src/mem/ruby/structures/RubyMemoryControl.hh 9689ead7b479 > src/mem/ruby/structures/RubyMemoryControl.cc 9689ead7b479 > src/mem/slicc/ast/FuncCallExprAST.py 9689ead7b479 > src/mem/slicc/ast/IfStatementAST.py 9689ead7b479 > src/mem/slicc/ast/InPortDeclAST.py 9689ead7b479 > src/mem/slicc/symbols/StateMachine.py 9689ead7b479 > src/mem/ruby/common/Consumer.hh 9689ead7b479 > src/mem/ruby/common/Consumer.cc 9689ead7b479 > src/mem/ruby/network/MessageBuffer.hh 9689ead7b479 > src/mem/ruby/network/MessageBuffer.cc 9689ead7b479 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 9689ead7b479 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 9689ead7b479 > src/mem/ruby/network/simple/PerfectSwitch.hh 9689ead7b479 > > Diff: http://reviews.gem5.org/r/2987/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
