From: David Lin <davidzylin@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, davidlin@stripe.com, davidzylin@gmail.com,
ps@pks.im, stolee@gmail.com
Subject: [PATCH v2] cache-tree: fix inverted object existence check in cache_tree_fully_valid
Date: Mon, 6 Apr 2026 15:27:11 -0400 [thread overview]
Message-ID: <20260406192711.68870-1-davidlin@stripe.com> (raw)
In-Reply-To: <xmqqcy0bc4bf.fsf@gitster.g>
The negation in front of the object existence check in
cache_tree_fully_valid() was lost in 062b914c84 (treewide: convert
users of `repo_has_object_file()` to `has_object()`, 2025-04-29),
turning `!repo_has_object_file(...)` into `has_object(...)` instead
of `!has_object(...)`.
This makes cache_tree_fully_valid() always report the cache tree as
invalid when objects exist (the common case), forcing callers like
write_index_as_tree() to call cache_tree_update() on every
invocation. An odb_has_object() check inside update_one() avoids a
full tree rebuild, but the unnecessary call still pays the cost of
opening an ODB transaction and, in partial clones, a promisor remote
check.
Restore the missing negation and add a test that verifies write-tree
takes the cache-tree shortcut when the cache tree is valid.
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: David Lin <davidlin@stripe.com>
---
Notes:
Changes since v1:
- Use test_grep instead of bare grep (Stolee)
cache-tree.c | 2 +-
t/t0090-cache-tree.sh | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/cache-tree.c b/cache-tree.c
index 60bcc07c3b..9fe057355c 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -238,7 +238,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
if (!it)
return 0;
if (it->entry_count < 0 ||
- odb_has_object(the_repository->objects, &it->oid,
+ !odb_has_object(the_repository->objects, &it->oid,
HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index d901588294..0964718d7f 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -278,4 +278,12 @@ test_expect_success 'switching trees does not invalidate shared index' '
)
'
+test_expect_success 'cache-tree is used by write-tree when valid' '
+ test_commit use-valid &&
+
+ # write-tree with a valid cache-tree should skip cache_tree_update
+ GIT_TRACE2_PERF="$(pwd)/trace.output" git write-tree &&
+ test_grep ! region_enter.*cache_tree.*update trace.output
+'
+
test_done
base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
--
2.52.0.ge17bebe515.stripe
next prev parent reply other threads:[~2026-04-06 19:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-06 15:14 [PATCH] cache-tree: fix inverted object existence check in cache_tree_fully_valid David Lin
2026-04-06 18:10 ` Derrick Stolee
2026-04-06 18:49 ` Junio C Hamano
2026-04-06 19:27 ` David Lin [this message]
2026-04-07 5:15 ` [PATCH v2] " Patrick Steinhardt
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=20260406192711.68870-1-davidlin@stripe.com \
--to=davidzylin@gmail.com \
--cc=davidlin@stripe.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ps@pks.im \
--cc=stolee@gmail.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.