Sorry for the slow reply. I still like the idea of making these features implicitly available by building this into MasterPort directly, but I see where there would be complications. I'm OK with committing it as is and leaving the integration into MasterPort as a future desired feature ;-).
Steve On Tue, May 1, 2012 at 9:33 AM, Andreas Hansson <[email protected]>wrote: > Hi Steve,**** > > ** ** > > Thanks for the feedback. After a bit of thinking, the fundamental problem > with doing this “implicitly” as you suggest is that it has to be done per > port, and not per MemObject. Thus, the enable/disable would have to be > moved to each and every port, and the ports are parameters in the Python > classes, not SimObjects/MemObjects, so it is not straight forward to > enable/disable the stats in a convenient way (unless we start turning the > flags in to vector params etc). Similarly for the stats, they would have to > be stats implemented in each port, or some for of multi-dimensional stats > on the MemObjects. **** > > ** ** > > I think the functionality and configuration interface ends up being rather > cluttered, but haven’t actually attempted to implement it.**** > > ** ** > > In conclusion, I think there are compelling reasons to keep the monitor > the way it is.**** > > ** ** > > Andreas**** > > ** ** > > *From:* Steve Reinhardt [mailto:[email protected]] > *Sent:* 30 April 2012 01:08 > *To:* Andreas Hansson > *Cc:* Default > *Subject:* Re: Review Request: MEM: Add the communication monitor**** > > ** ** > > (switching to email since this is getting a bit detailed)**** > > ** ** > > These are valid concerns. I'd like to explore them a little more. I'm > not necessarily picking sides, just playing devil's advocate a bit.**** > > On Sun, Apr 29, 2012 at 5:05 AM, Andreas Hansson <[email protected]> > wrote:**** > > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1159/ **** > > ** ** > > On April 28th, 2012, 9:08 a.m., *Steve Reinhardt* wrote:**** > > I like the idea of having a common way of collecting these stats (and > probably more importantly, common names and semantics for the output). Does > it make sense to have this in a separate object though? It seems like that > just adds complexity to the configuration, and the possibility of not putting > one of these where you wish you had. Did you consider putting all this code > in a base class like MemObject so that everyone automatically collects all > these stats?**** > > A very valid comment, and this option was under serious consideration. The > benefits of having it behaviourally encapsulated (as you now propose) rather > than structurally encapsulated (as the current patch does) is indeed that:**** > > 1) it does not involve changing any configuration scripts, and **** > > 2) any communicating object would have a uniform set of stats relating to the > communication**** > > ** ** > > The issues are:**** > > 1) how to "hide" it in the interface timing calls, as we must catch the case > where sendTiming returns false and undo the changes to the senderState**** > > If we make this common to all MemObjects, we could take the monitor > state (which is just a tick value) and put it in the base SenderState > class. I bet there are some MemObjects that already record the transmit > time in the sender state anyway so we could get rid of that double > recording. Eliminating the memory allocation and chaining of the separate > SenderState object could possibly be a noticeable performance win when > you're actually collecting the stats.**** > > ** ** > > 2) the rather extensive set of disable flags in the object parameters**** > > This doesn't bother me much. If we define the flags on the base > MemObject and provide reasonable defaults, they'll be invisible to most > people. **** > > ** ** > > 3) the potential negative performance impact caused by collecting more stats > than we really want**** > > How expensive are these stats? A timing-mode transaction is not cheap > to begin with, so the percentage slowdown may not be that bad. Plus I find > that, when you're running long simulations, you're better off collecting > all the stats you can (within reason), since having them and not needing > them is less costly than wanting them and not having them.**** > > ** ** > > Plus, it looks like all the stats can be disabled, so if performance is a > concern, they could just be turned off, right? This would eliminate the > first-order overheads (notwithstanding #5 below).**** > > ** ** > > 4) more "noise" in the stats output**** > > The stats output really isn't designed to be read casually anyway... if > you're using grep or some other script, then the excess data won't be > noticeable. **** > > ** ** > > 5) potential negative performance impact even when not collecting stats due > to the large number of if-statements**** > > True. As with #3, we don't know how big this impact is though. > Branch predictors can be pretty good, so although a sequence of > load/compare/branch instructions isn't free, it's not necessarily as bad as > you think.**** > > ** ** > > Plus, there may be some existing stats in some MemObjects that are > redundant with respect to these (like bytes read and bytes written maybe?) > and if we go back and get rid of those then we'd save a little overhead > there.**** > > ** ** > > The current CommMonitor may not be ideal in that it requires config script > changes, and it is indeed up to the user to determine which points in the > memory system are of interest. However, I would expect no more than a handful > monitors in a typical system, and the stats in the monitor could become > multi-dimensional and indexed on the masterId in a future patch.**** > > ** ** > > How many masters are there in a typical system anyway, and how many of > them (1) issue a lot of requests and (2) are ones we're not likely to care > about stats on? These are the only cases where it would matter to not > collect stats, IMO. My guess is that the only things that might fall into > this category are fabric components like the Bus and *maybe* the Bridge. I > think the CPUs are the things you would definitely want to be capturing > stats on, and other things like DMA devices probably don't issue enough > requests typically to matter (from a simulation performance perspective).* > *** > > **** > > ** ** > > As a minor side note, the structural CommMonitor also more closely resembles > the bus monitors used in actual devices.**** > > ** ** > > Only because in silicon it's too expensive to automatically put one in > every bus master ;-). In software, it's not so bad...**** > > ** ** > > As I said, I'm not convinced either way at this point, just trying to > explore the issues a little more deeply. Thanks for bearing with me.**** > > ** ** > > Steve**** > > **** > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
