> On Aug. 14, 2015, 8:34 p.m., Joel Hestness wrote: > > Nice work tracking down 15-20% sim time! I WANT THAT! > > > > It seems like this patch is trying to do a couple fairly orthogonal things: > > (1) improve performance by removing the Consumer class' scheduled event > > lookups and dynamically allocated events, and (2) move wakeup event > > scheduling responsibility from the Consumers into MessageBuffers. I really > > like the first aim, but I don't agree with the second. More below:
The patch will ultimately eliminate the Consumer class. So both (1) and (2) will happen. (2) is incomplete in the sense that the responsibility is being shared by both the message buffer and the receiver. > On Aug. 14, 2015, 8:34 p.m., Joel Hestness wrote: > > src/mem/ruby/common/Consumer.cc, line 46 > > <http://reviews.gem5.org/r/2987/diff/1/?file=48400#file48400line46> > > > > Is the majority of the performance gain just from removing the > > alreadyScheduled() calls and dynamically allocated events? Or something > > else? > > > > If the gain is from axing alreadyScheduled() and dynamic events, then > > that's all this patch should change. The majority gain is from making the events fine grained. > On Aug. 14, 2015, 8:34 p.m., Joel Hestness wrote: > > src/mem/ruby/network/MessageBuffer.cc, line 143 > > <http://reviews.gem5.org/r/2987/diff/1/?file=48402#file48402line143> > > > > Why does the MessageBuffer now have a pointer to the receiver's > > wakeupEvent? > > > > I feel that it be the responsibility of the Consumer (receiver) to > > figure out when it will be able to pop something from the buffer. The > > MessageBuffer sorts its in-flight messages by time in order to signal to > > the receiver the first time at which the receiver is allowed to pop from > > the buffer. Seems like it would make more sense for the MessageBuffer to > > call a function on the receiver pointer to tell it when it can do so (e.g. > > scheduleEventAbsolute). The receiver has multiple wakeup events, one per incoming message buffer. So more information needs to be provided by the message buffer on what event to be scheduled. That information is the pointer to the wakeup event corresponding to this particular message buffer. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2987/#review6956 ----------------------------------------------------------- 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
