On Wed, Jun 12, 2013 at 11:38:41AM -0700, Alex Wang wrote: > This commit changes the "ovs-appclt coverage/show" command to show the > per-second, per-minute and per-hour rates of function invocation. More > importantly, this makes using coverage counter an easy way to monitor > the execution of specific functions. > > Signed-off-by: Alex Wang <al...@nicira.com> > --- > Possible Issue: > > The "coverage_run" runs every second. And the number of addition operations is > not very large. If it is still considered to be time-consuming, I'll adjust > further. >
I have two high-level design questions. First is the cost, not of the periodic summations (I doubt that cost matters) but of forcing a wakeup every second. This probably doesn't matter in ovs-vswitchd, but other daemons that don't wake up so frequently also use the poll_loop library, and it would be a shame to drive up system load unnecessarily. So I would prefer, if possible, for the coverage code not to use a timer but instead to just do work in coverage_run() whenever the process happens to wake up. Second is accuracy. I believe that the current code considers any interval greater than or equal to a second as exactly a second. Because wakeups aren't precise, that tends to stretch out the definition of a second to somewhat longer. If you take my suggestion about cost, above, that makes the problem worse. So I think that it may be worthwhile to measure the length of the actual interval for a "second" and divide by its length. For example, if there were 100 events of some type over 1.5 seconds, we would count that as 100 / 1.5 == 66.67 events per second, and use that in the average. That might require using "double"s for the average counters, or we could use fixed-point arithmetic; I think either solution could be OK. I didn't do a detailed review yet because I'd like to hear your thoughts on these points first. I did notice that coverage_array_sum() is only used in coverage.c, so I would define it there (and not "inline"), not in coverage.h. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
