Re: [PATCH v2] index: add trace2 region for clear skip worktree

From

Junio C Hamano <gitster@pobox.com>

Date

Oct 28, 2022, 17:17 UTC

Message-ID

<xmqqy1szj29n.fsf@gitster.g>

In-Reply-To

<313a6619-273c-066e-c3da-a519b38c03af@github.com>

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.

 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);
 }
 
 /*