On Thu, Mar 10, 2016 at 7:05 AM, David Turner <dturner@xxxxxxxxxxxxxxxx> wrote: > On Wed, 2016-03-09 at 14:58 -0800, Junio C Hamano wrote: >> David Turner <dturner@xxxxxxxxxxxxxxxx> writes: > ... >> > trace_stats() is intended for GIT_TRACE_*_STATS variable group and >> > GIT_TRACE_PACK_STATS is more like an example how new vars can be >> > added. > ... > >> > + pack_access_nr++; >> > } >> >> This looks rather half-hearted, in that those who are interested in >> this new number can run "wc -l" on the pack-access trace log without >> adding a new "stats" anyway. It may make the "stats" far more >> useful to keep track of the summary information of what would be >> written to the pack access log and add to the report_pack_stats() >> output things like the average and median distance of seeks >> (i.e. differences in the "obj_offset" into the same pack by two >> adjacent pack accesse) and the variance, for example? > > On reflection, I do not think I need this patch; it was in Duy's series > and the index stats patch would need to be rewritten slightly to get > rid of it, but I'm OK with that. I wanted to make minimal changes to > Duy's patches, but I'd rather make larger changes than do work on > patches I don't need. > > So unless Duy objects, I'm going to drop this from the series. I wanted something to verify from the tests and this was the best I could come up. But since I added trace_printf_key(&trace_exclude,... I think that could serve better to show what's going on, and we can easily summarize from the trace if we want to. That's one option. If you can adjust the tests another way, I'm fine too. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html