All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 1/2] core: use env variable instead of config var to turn on logging pack access
Date: Sun,  9 Jun 2013 12:22:48 +0700	[thread overview]
Message-ID: <1370755369-30868-1-git-send-email-pclouds@gmail.com> (raw)

5f44324 (core: log offset pack data accesses happened - 2011-07-06)
provides a way to observe pack access patterns via a config
switch. Setting an environment variable looks more obvious than a
config var, especially when you just need to _observe_, and more
inline with other tracing knobs we have.

Document it as it may be useful for remote troubleshooting.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git.txt |  7 +++++++
 cache.h               |  3 ---
 config.c              |  3 ---
 environment.c         |  1 -
 sha1_file.c           | 14 ++++++++++++--
 5 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 68f1ee6..c760918 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -838,6 +838,13 @@ for further details.
 	as a file path and will try to write the trace messages
 	into it.
 
+'GIT_TRACE_PACK_ACCESS'::
+	If this variable is set to a path, a file will be created at
+	the given path logging all accesses to any packs. For each
+	access, the pack file name and an offset in the pack is
+	recorded. This may be helpful for troubleshooting some
+	pack-related performance problems.
+
 GIT_LITERAL_PATHSPECS::
 	Setting this variable to `1` will cause Git to treat all
 	pathspecs literally, rather than as glob patterns. For example,
diff --git a/cache.h b/cache.h
index df532f8..4f41606 100644
--- a/cache.h
+++ b/cache.h
@@ -772,9 +772,6 @@ extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
 /* global flag to enable extra checks when accessing packed objects */
 extern int do_check_packed_object_crc;
 
-/* for development: log offset of pack access */
-extern const char *log_pack_access;
-
 extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type);
 
 extern int move_temp_to_file(const char *tmpfile, const char *filename);
diff --git a/config.c b/config.c
index 7a85ebd..d04e815 100644
--- a/config.c
+++ b/config.c
@@ -688,9 +688,6 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
-	if (!strcmp(var, "core.logpackaccess"))
-		return git_config_string(&log_pack_access, var, value);
-
 	if (!strcmp(var, "core.autocrlf")) {
 		if (value && !strcasecmp(value, "input")) {
 			if (core_eol == EOL_CRLF)
diff --git a/environment.c b/environment.c
index e2e75c1..0cb67b2 100644
--- a/environment.c
+++ b/environment.c
@@ -37,7 +37,6 @@ size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
 size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
 size_t delta_base_cache_limit = 16 * 1024 * 1024;
 unsigned long big_file_threshold = 512 * 1024 * 1024;
-const char *log_pack_access;
 const char *pager_program;
 int pager_use_color = 1;
 const char *editor_program;
diff --git a/sha1_file.c b/sha1_file.c
index b114cc9..4b23bb8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -36,6 +36,9 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
 
 const unsigned char null_sha1[20];
 
+static const char *no_log_pack_access = "no_log_pack_access";
+static const char *log_pack_access;
+
 /*
  * This is meant to hold a *small* number of objects that you would
  * want read_sha1_file() to be able to return, but yet you do not want
@@ -1956,12 +1959,19 @@ static void write_pack_access_log(struct packed_git *p, off_t obj_offset)
 {
 	static FILE *log_file;
 
+	if (!log_pack_access)
+		log_pack_access = getenv("GIT_TRACE_PACK_ACCESS");
+	if (!log_pack_access)
+		log_pack_access = no_log_pack_access;
+	if (log_pack_access == no_log_pack_access)
+		return;
+
 	if (!log_file) {
 		log_file = fopen(log_pack_access, "w");
 		if (!log_file) {
 			error("cannot open pack access log '%s' for writing: %s",
 			      log_pack_access, strerror(errno));
-			log_pack_access = NULL;
+			log_pack_access = no_log_pack_access;
 			return;
 		}
 	}
@@ -1992,7 +2002,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 	int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC;
 	int base_from_cache = 0;
 
-	if (log_pack_access)
+	if (log_pack_access != no_log_pack_access)
 		write_pack_access_log(p, obj_offset);
 
 	/* PHASE 1: drill down to the innermost base object */
-- 
1.8.2.83.gc99314b

             reply	other threads:[~2013-06-09  5:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-09  5:22 Nguyễn Thái Ngọc Duy [this message]
2013-06-09  5:22 ` [PATCH 2/2] git.txt: document GIT_TRACE_PACKET Nguyễn Thái Ngọc Duy
2013-06-09  5:36   ` Jeff King
2013-06-09  5:53     ` Nguyễn Thái Ngọc Duy
2013-06-09  5:55       ` Jeff King
2013-06-09 23:16         ` Junio C Hamano
2013-06-09  7:20       ` Eric Sunshine
2013-06-09  7:17 ` [PATCH 1/2] core: use env variable instead of config var to turn on logging pack access Eric Sunshine
2013-06-09  7:18 ` Eric Sunshine
  -- strict thread matches above, loose matches on Subject: below --
2013-05-30 13:51 Nguyễn Thái Ngọc Duy
2013-06-03 20:24 ` Junio C Hamano

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=1370755369-30868-1-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.