[varnishstat] reset running averages #4274
Conversation
I've been running benchmark tests these last few days, and I've been wishing to be able to reset the average calculations without having to restart `varnishncsa`, so here we go.
dridi
changed the title
dridi
changed the title
|
Please remove all mentions to |
|
Ah yes, of course |
|
Please also make it two commits, it's easier to back-port to any branch without the guaranteed conflict in the changelog. |
|
I accidentally submitted the previous comment, I meant to also ask for basic test coverage. |
| sample_points(); | ||
| sample_hitrate(); | ||
| sample_points(reset_averages); | ||
| sample_hitrate(reset_averages); | ||
| reset_averages = 0; |
There was a problem hiding this comment.
Why pass reset_averages to the sample_*() functions?
There was a problem hiding this comment.
this was a quick 10 minutes job, and I assumed the key presses could come at any given time and wanted to avoid different counters from being handled differently, but I see the wgetch() call now, I can indeed use the global variable
There was a problem hiding this comment.
It's single-threaded, you are fine with globals :)
| if (reset) { | ||
| hitrate.hr_10.n = 0; | ||
| hitrate.hr_100.n = 0; | ||
| hitrate.hr_1000.n = 0; | ||
| } | ||
| update_ma(&hitrate.hr_10, ratio); | ||
| update_ma(&hitrate.hr_100, ratio); | ||
| update_ma(&hitrate.hr_1000, ratio); |
There was a problem hiding this comment.
Here you could use reset_averages directly.
| if (pt->t_last) | ||
| if (reset) { | ||
| pt->chg = 0; | ||
| pt->ma_10.n = 0; | ||
| pt->ma_100.n = 0; | ||
| pt->ma_1000.n = 0; | ||
| } else if (pt->t_last) | ||
| pt->chg = ((int64_t)pt->cur - (int64_t)pt->last) / | ||
| (pt->t_cur - pt->t_last); |
There was a problem hiding this comment.
To be consistent with the block of code from my previous comment, shouldn't it look like this?
if (reset) { // or better: if (reset_averages)
pt->chg = 0;
pt->ma_10.n = 0;
pt->ma_100.n = 0;
pt->ma_1000.n = 0;
}
if (pt->t_last) {
pt->chg = ((int64_t)pt->cur - (int64_t)pt->last) /
(pt->t_cur - pt->t_last);
}For the hit ratio you reset the 10/100/1000 averages and let them be updated in the same iteration. So for consistency you should give the counters a chance to be updated too.
bsdphk
changed the title
|
I like phk's idea to use the |
I've been running benchmark tests these last few days, and I've been wishing to be able to reset the average calculations without having to restart
varnishncsa, so here we go.