Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Derrick Stolee <derrickstolee@github.com>
Cc: Anh Le via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Timothy Jones <timothy@canva.com>,
	Jeff Hostetler <jeffhost@microsoft.com>,
	Jeff Hostetler <git@jeffhostetler.com>, Anh Le <anh@canva.com>
Subject: Re: [PATCH v2] index: add trace2 region for clear skip worktree
Date: Fri, 28 Oct 2022 10:17:56 -0700	[thread overview]
Message-ID: <xmqqy1szj29n.fsf@gitster.g> (raw)
In-Reply-To: <313a6619-273c-066e-c3da-a519b38c03af@github.com> (Derrick Stolee's message of "Fri, 28 Oct 2022 11:49:20 -0400")

Derrick Stolee <derrickstolee@github.com> writes:

> A few style things:
>
> 1. Use "if (path_counts[0])" to detect a non-zero value.
> 2. Don't use { } around a single-line block.
> 3. Your lines are quite long, due a lot from your long keys.
>    Shorten the keys (maybe "sparse_path_count" and "restarted_count"
>    is enough context) and consider using a line break in
>    the middle of the parameters.

All good suggestions.  Let me add another one.

  4. Call an array you primarily access its individual elements in
     singular.  That way, you can say path_count[0] (i.e. the count
     for the zeroth round).  An array that exists mostly to be
     passed around as a whole to represent a "set of things", on the
     other hand, can be called in plural.

Taking them all together, perhaps something like this is what you
meant?

 sparse-index.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git c/sparse-index.c w/sparse-index.c
index dbf647949c..8c269dab80 100644
--- c/sparse-index.c
+++ w/sparse-index.c
@@ -493,22 +493,25 @@ void clear_skip_worktree_from_present_files(struct index_state *istate)
 	int dir_found = 1;
 
 	int i;
-	int path_counts[2] = {0, 0};
+	int path_count[2] = {0, 0};
 	int restarted = 0;
 
 	if (!core_apply_sparse_checkout ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
-	trace2_region_enter("index", "clear_skip_worktree_from_present_files", istate->repo);
+	trace2_region_enter("index", "clear_skip_worktree_from_present_files",
+			    istate->repo);
 restart:
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct cache_entry *ce = istate->cache[i];
 
 		if (ce_skip_worktree(ce)) {
-			path_counts[restarted]++;
+			path_count[restarted]++;
 			if (path_found(ce->name, &last_dirname, &dir_len, &dir_found)) {
 				if (S_ISSPARSEDIR(ce->ce_mode)) {
+					if (restarted)
+						BUG("ensure-full-index did not fully flatten?");
 					ensure_full_index(istate);
 					restarted = 1;
 					goto restart;
@@ -518,13 +521,14 @@ void clear_skip_worktree_from_present_files(struct index_state *istate)
 		}
 	}
 
-	if (path_counts[0] > 0) {
-		trace2_data_intmax("index", istate->repo, "clear_skip_worktree_from_present_files/path_count", path_counts[0]);
-	}
-	if (restarted) {
-		trace2_data_intmax("index", istate->repo, "clear_skip_worktree_from_present_files/full_index/path_count", path_counts[1]);
-	}
-	trace2_region_leave("index", "clear_skip_worktree_from_present_files", istate->repo);
+	if (path_count[0])
+		trace2_data_intmax("index", istate->repo,
+				   "sparse_path_count", path_count[0]);
+	if (restarted)
+		trace2_data_intmax("index", istate->repo,
+				   "sparse_path_count_full", path_count[1]);
+	trace2_region_leave("index", "clear_skip_worktree_from_present_files",
+			    istate->repo);
 }
 
 /*


  reply	other threads:[~2022-10-28 17:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-26  0:05 [PATCH] index: add trace2 region for clear skip worktree Anh Le via GitGitGadget
2022-10-26  3:16 ` Junio C Hamano
2022-10-26 14:13   ` Jeff Hostetler
2022-10-26 16:01     ` Junio C Hamano
2022-10-26 18:29       ` Jeff Hostetler
2022-10-27  0:04         ` Anh Le
2022-10-28  0:46 ` [PATCH v2] " Anh Le via GitGitGadget
2022-10-28 15:49   ` Derrick Stolee
2022-10-28 17:17     ` Junio C Hamano [this message]
2022-10-30 23:28       ` Anh Le
2022-10-28 16:50   ` Jeff Hostetler
2022-10-31  0:56   ` [PATCH v3] " Anh Le via GitGitGadget
2022-10-31 22:34     ` Taylor Blau
2022-11-03 23:04     ` [PATCH v4 0/2] " Anh Le via GitGitGadget
2022-11-03 23:05       ` [PATCH v4 1/2] " Anh Le via GitGitGadget
2022-11-03 23:05       ` [PATCH v4 2/2] index: raise a bug if the index is materialised more than once Anh Le via GitGitGadget
2022-11-05  0:29       ` [PATCH v4 0/2] index: add trace2 region for clear skip worktree Taylor Blau
2022-11-07 20:50         ` Derrick Stolee

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=xmqqy1szj29n.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=anh@canva.com \
    --cc=derrickstolee@github.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jeffhost@microsoft.com \
    --cc=timothy@canva.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).