Re: [PATCH 01/19] trace.c: add GIT_TRACE_PACK_STATS for pack usage statistics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



David Turner <dturner@xxxxxxxxxxxxxxxx> writes:

> From: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>
> 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.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  Documentation/git.txt |  3 +++
>  cache.h               |  2 ++
>  git.c                 |  1 +
>  sha1_file.c           | 24 ++++++++++++++++++++++++
>  trace.c               | 13 +++++++++++++
>  trace.h               |  1 +
>  6 files changed, 44 insertions(+)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 2754af8..794271e 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -1055,6 +1055,9 @@ of clones and fetches.
>  	time of each Git command.
>  	See 'GIT_TRACE' for available trace output options.
>  
> +'GIT_TRACE_PACK_STATS'::
> +	Print various statistics.

"Various" is quite vague.

Perhaps a new description in Documentation/technical/ might be a
good thing to have?

> +void report_pack_stats(struct trace_key *key)
> +{
> +	trace_printf_key(key, "\n"
> +			 "pack_report: getpagesize()            = %10" SZ_FMT "\n"
> +			 "pack_report: core.packedGitWindowSize = %10" SZ_FMT "\n"
> +			 "pack_report: core.packedGitLimit      = %10" SZ_FMT "\n"
> +			 "pack_report: pack_used_ctr            = %10u\n"
> +			 "pack_report: pack_mmap_calls          = %10u\n"
> +			 "pack_report: pack_open_windows        = %10u / %10u\n"
> +			 "pack_report: pack_mapped              = "
> +			 "%10" SZ_FMT " / %10" SZ_FMT "\n"
> +			 "pack_report: pack accesss             = %10u\n",
> +			 sz_fmt(getpagesize()),
> +			 sz_fmt(packed_git_window_size),
> +			 sz_fmt(packed_git_limit),
> +			 pack_used_ctr,
> +			 pack_mmap_calls,
> +			 pack_open_windows, peak_pack_open_windows,
> +			 sz_fmt(pack_mapped), sz_fmt(peak_pack_mapped),
> +			 pack_access_nr);
> +}
> +
>  /*
>   * Open and mmap the index file at path, perform a couple of
>   * consistency checks, then record its information to p.  Return 0 on
> @@ -2238,6 +2261,7 @@ static void write_pack_access_log(struct packed_git *p, off_t obj_offset)
>  	static struct trace_key pack_access = TRACE_KEY_INIT(PACK_ACCESS);
>  	trace_printf_key(&pack_access, "%s %"PRIuMAX"\n",
>  			 p->pack_name, (uintmax_t)obj_offset);
> +	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?
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]