public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: David Turner <dturner@twopensource•com>
Cc: git@vger•kernel.org, pclouds@gmail•com
Subject: Re: [PATCH 01/19] trace.c: add GIT_TRACE_PACK_STATS for pack usage statistics
Date: Wed, 09 Mar 2016 14:58:12 -0800	[thread overview]
Message-ID: <xmqq4mcfgsxn.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1457548582-28302-2-git-send-email-dturner@twopensource.com> (David Turner's message of "Wed, 9 Mar 2016 13:36:04 -0500")

David Turner <dturner@twopensource•com> writes:

> From: Nguyễn Thái Ngọc Duy <pclouds@gmail•com>
>
> 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@gmail•com>
> ---
>  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?

  reply	other threads:[~2016-03-09 22:58 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-09 18:36 [PATCH 00/19] index-helper, watchman David Turner
2016-03-09 18:36 ` [PATCH 01/19] trace.c: add GIT_TRACE_PACK_STATS for pack usage statistics David Turner
2016-03-09 22:58   ` Junio C Hamano [this message]
2016-03-10  0:05     ` David Turner
2016-03-10 10:59       ` Duy Nguyen
2016-03-09 18:36 ` [PATCH 02/19] read-cache.c: fix constness of verify_hdr() David Turner
2016-03-09 18:36 ` [PATCH 03/19] read-cache: allow to keep mmap'd memory after reading David Turner
2016-03-09 23:02   ` Junio C Hamano
2016-03-10  0:09     ` David Turner
2016-03-09 18:36 ` [PATCH 04/19] index-helper: new daemon for caching index and related stuff David Turner
2016-03-09 23:09   ` Junio C Hamano
2016-03-09 23:21     ` Junio C Hamano
2016-03-10  0:01       ` David Turner
2016-03-10 11:17       ` Duy Nguyen
2016-03-10 20:22         ` David Turner
2016-03-11  1:11           ` Duy Nguyen
2016-03-10  0:18     ` David Turner
2016-03-15 11:56     ` Duy Nguyen
2016-03-15 15:56       ` Junio C Hamano
2016-03-15 11:52   ` Duy Nguyen
2016-03-09 18:36 ` [PATCH 05/19] trace.c: add GIT_TRACE_INDEX_STATS for index statistics David Turner
2016-03-09 18:36 ` [PATCH 06/19] index-helper: add --strict David Turner
2016-03-09 18:36 ` [PATCH 07/19] daemonize(): set a flag before exiting the main process David Turner
2016-03-09 18:36 ` [PATCH 08/19] index-helper: add --detach David Turner
2016-03-09 18:36 ` [PATCH 09/19] index-helper: add Windows support David Turner
2016-03-16 11:42   ` Duy Nguyen
2016-03-17 12:18     ` Johannes Schindelin
2016-03-17 12:59       ` Duy Nguyen
2016-03-09 18:36 ` [PATCH 10/19] read-cache: add watchman 'WAMA' extension David Turner
2016-03-09 18:36 ` [PATCH 11/19] Add watchman support to reduce index refresh cost David Turner
2016-03-09 18:36 ` [PATCH 12/19] read-cache: allow index-helper to prepare shm before git reads it David Turner
2016-03-09 18:36 ` [PATCH 13/19] index-helper: use watchman to avoid refreshing index with lstat() David Turner
2016-03-09 18:36 ` [PATCH 14/19] update-index: enable/disable watchman support David Turner
2016-03-09 18:36 ` [PATCH 15/19] unpack-trees: preserve index extensions David Turner
2016-03-09 18:36 ` [PATCH 16/19] index-helper: rewrite pidfile after daemonizing David Turner
2016-03-09 18:36 ` [PATCH 17/19] index-helper: process management David Turner
2016-03-09 18:36 ` [PATCH 18/19] index-helper: autorun David Turner
2016-03-15 12:12   ` Duy Nguyen
2016-03-15 14:26     ` Johannes Schindelin
2016-03-16 11:37       ` Duy Nguyen
2016-03-16 18:11       ` David Turner
2016-03-16 18:27         ` Johannes Schindelin
2016-03-17 13:02           ` Duy Nguyen
2016-03-17 14:43             ` Johannes Schindelin
2016-03-17 18:31               ` David Turner
2016-03-18  0:50               ` Duy Nguyen
2016-03-18  7:14                 ` Johannes Schindelin
2016-03-18  7:44                   ` Duy Nguyen
2016-03-18 17:22                     ` David Turner
2016-03-18 23:09                       ` Duy Nguyen
2016-03-18  7:17                 ` Johannes Schindelin
2016-03-18  7:34                   ` Duy Nguyen
2016-03-18 15:57                     ` Johannes Schindelin
2016-03-09 18:36 ` [PATCH 19/19] hack: watchman/untracked cache mashup David Turner
2016-03-15 12:31   ` Duy Nguyen
2016-03-17  0:56     ` David Turner
2016-03-17 13:06       ` Duy Nguyen
2016-03-17 18:08         ` David Turner
2016-03-29 17:09 ` [PATCH 00/19] index-helper, watchman Torsten Bögershausen
2016-03-29 21:51   ` David Turner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq4mcfgsxn.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=dturner@twopensource$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox