Thanks Ben,
On Wed, Jun 12, 2013 at 4:20 PM, Ben Pfaff <b...@nicira.com> wrote: > 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. > This is really important to know. > 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. > Yes, this will achieve the per-second rate. But will not be very easy to compute the per-minute and hourly rate. So far, I cannot find a very efficient way to calculate these (maybe use long linked-lists that count to 1 minute and 1 hour?). I'll put detailed comments in v2. 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 for pointing this out. > Thanks, > > Ben.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
