send-pack: avoid resending unchanged objects after rebasing

A common feature-branch workflow is to push a branch, rebase it onto an updated upstream, and force-push the rewritten commits. Large blobs added or changed on the feature branch may survive the rebase unchanged. Although the receiver already has those blobs through the old branch tip, Git can pack and send them again. This can turn a small history rewrite into a large upload.

This series teaches send-pack to request inspection of the old tip's tree before packing, so more receiver-known objects can be excluded.

The additional inspection is enabled only for forced non-fast-forward updates of existing branches with locally available old tips. Ordinary fast-forward pushes do not enable it, including ones made with --force. No server or wire-protocol change is needed.

The old tip is already a negative revision, but it may lie outside the boundary visited by the ordinary revision walk. Its tree is then not necessarily inspected. Passing it to pack-objects as --inspect-have lets us compare its root tree with the outgoing trees using the existing sparse tree marking. This excludes additional receiver-known objects without walking the old tip's commit history, and prunes identical and receiver-only subtrees.

Shallow pushes keep their existing aggressive edge handling. Partial clones skip the extra tree inspection to avoid fetching missing objects, and successful bitmap enumeration keeps its existing path. Old tips are resolved in pack-objects, where replacement refs are already disabled.

I compared upstream 7e34d8174dcda3435d1404c9a30bef46afe6c13e with the patched source using trees from Chromium commit 5b05cc16e960a021389ef9b543ab38a037c38d60. A depth-two clone supplies the objects; the measured fixture has a complete, non-shallow history with a common base and sibling receiver/outgoing commits.

Both builds used GCC 15.2.0 and -g -O2 -Wall on an Intel i7-13700H under WSL2. Measurements used a warm cache, sparse enumeration, no bitmaps, one packing thread, two warm-up pairs and 15 measured pairs with alternating build order. Values below are upstream/patched; total wall times are medians in milliseconds, including process startup.

Case Objects Pack bytes Total wall ms Reword 3 / 1 421 / 215 2.776 / 1.338 Single file 6 / 5 667 / 553 3.542 / 2.744 ~99% of files 552,724 / 552,723 58,082,988 / 58,082,873 4,525.094 / 4,883.623

In the single-file case, enumeration increases from 0.758 to 0.982 ms, but pack preparation decreases from 1.637 to 0.458 ms. In the last case, 506,468 regular files receive distinct small replacement blobs; total packing time increases by about 8%, saving only 115 pack bytes.

These are isolated local pack-objects measurements, not end-to-end pushes:
network transfer and receiving-server work are excluded. Where redundant large blobs materially inflate the pack, avoiding their transfer should further reduce end-to-end push time, especially on slower connections; that benefit was not measured here.

Most objects in this fixture are already excluded through the common base, so the pack-size savings are small. The widespread-change case tests inspection overhead on Chromium's directory structure, rather than realistic compression of a whole-project rewrite. Feedback on this cost and the scope of the inspection would be particularly useful.

Local verification passed t5322-pack-objects-sparse.sh (19 tests) and t5516-fetch-push.sh (132 tests). Coverage includes identical trees, shared blobs under different trees, retained changed objects, subtree pruning, partial clones and local replacement refs.

Marc Strapetz (3):
t5516: record redundant blobs after rebasing
pack-objects: inspect selected haves for shared objects
send-pack: avoid resending unchanged blobs after rebasing

 Documentation/git-pack-objects.adoc |   7 ++
 builtin/pack-objects.c              |  81 ++++++++++++++-
 odb.c                               |   1 +
 odb.h                               |   8 ++
 odb/source-files.c                  |   3 +
 send-pack.c                         |  30 ++++--
 t/t5322-pack-objects-sparse.sh      | 148 ++++++++++++++++++++++++++++
 t/t5516-fetch-push.sh               |  43 ++++++++
 8 files changed, 314 insertions(+), 7 deletions(-)

base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2281%2Fmstrap%2Fsend-pack-inspect-haves-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2281/mstrap/send-pack-inspect-haves-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2281

gitgitgadget

From: Marc Strapetz <[email protected]>

A forced push of a rebased branch can resend an unchanged blob even though the receiver has it through the advertised old tip.

Add object-count tests for --force, --force-with-lease and forced refspecs. Confirm that the blob is unchanged by the rebase and record the current three-object pack: the rebased commit, its tree and the unchanged blob. Disable push bitmaps to exercise normal enumeration.

Also cover local replacement refs, which must not cause objects missing from the receiver to be omitted.

---
 t/t5516-fetch-push.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

index b982b209bf..a46ba91ba0 100755
@@ -193,6 +193,49 @@ grep_wrote () {
 	grep 'write_pack_file/wrote.*"value":"'$1'"' $2
 }
 
+for push_args in \
+	"--force origin topic" \
+	"--force-with-lease origin topic" \
+	"origin +topic"
+do
+	test_expect_success "push of rebased branch with $push_args" '
+		mk_test testrepo heads/main &&
+		test_when_finished "rm -rf rebased rebased-push-event" &&
+		git clone testrepo rebased &&
+		(
+			cd rebased &&
+			git checkout -b topic &&
+			test_commit feature &&
+			git push origin topic &&
+			git checkout main &&
+			test_commit upstream path1 &&
+			git push origin main &&
+			git checkout topic &&
+			git rebase main &&
+			test_cmp_rev feature:feature.t HEAD:feature.t &&
+			GIT_TRACE2_EVENT="$D/rebased-push-event" \
+			git -c push.useBitmaps=false push $push_args
+		) &&
+		grep_wrote 3 rebased-push-event # 1 commit, 1 tree, 1 blob
+	'
+done
+
+test_expect_success 'forced update ignores replacement for old tip' '
+	mk_test testrepo heads/main &&
+	test_when_finished "rm -rf replaced" &&
+	git clone testrepo replaced &&
+	(
+		cd replaced &&
+		test_commit old &&
+		git push origin main &&
+		old_tip=$(git rev-parse HEAD) &&
+		git reset --hard HEAD^ &&
+		test_commit new &&
+		git replace "$old_tip" HEAD &&
+		git push --force origin main
+	)
+'
+
 test_expect_success 'push without negotiation' '
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&

gitgitgadget

From: Marc Strapetz <[email protected]>

A receiver-known commit can be excluded from a revision walk without becoming a boundary commit. Its tree then need not be inspected, so objects shared with the outgoing history can still be packed.

Accept --inspect-have <commit> lines in --revs input and carry the selected tips through the object database pack-generation options. Compare their root trees with the outgoing tips using the existing sparse tree marking. This identifies additional receiver-known objects without walking the selected commits' histories and avoids descending into identical or receiver-only subtrees.

Resolve the selected commits in pack-objects, where replacement refs are already disabled, so local replacements cannot substitute trees the receiver does not have. Keep successful bitmap enumeration on its existing path and skip the optional inspection in partial clones.

Cover identical root trees, shared blobs under different roots, retained changed objects, subtree pruning, non-commit input, partial clone fallback and shared objects in earlier outgoing commits.

---
 Documentation/git-pack-objects.adoc |   7 ++
 builtin/pack-objects.c              |  81 ++++++++++++++-
 odb.c                               |   1 +
 odb.h                               |   8 ++
 odb/source-files.c                  |   3 +
 t/t5322-pack-objects-sparse.sh      | 148 ++++++++++++++++++++++++++++
 6 files changed, 247 insertions(+), 1 deletion(-)

index ccad938c5b..a98d4286fa 100644
@@ -70,6 +70,13 @@ base-name::
 	outputs.  The objects on the resulting list are packed.
 	Besides revisions, `--not` or `--shallow <SHA-1>` lines are
 	also accepted.
++
+A `--inspect-have <commit>` line supplies the object ID of a commit known
+to the receiver. Before non-bitmap enumeration, its root tree is compared
+with the outgoing tips' root trees to identify additional objects that
+need not be packed, without walking its commit history. Identical
+subtrees and paths found only in the receiver-known trees are skipped.
+This optional inspection is skipped in partial clones.
 
 --unpacked::
 	This implies `--revs`.  When processing the list of

index 98c5c862f5..2bb27848ac 100644
@@ -731,6 +731,7 @@ void odb_generate_pack_options_release(struct odb_generate_pack_options *opts)
 {
 	oid_array_clear(&opts->wants);
 	oid_array_clear(&opts->haves);
+	oid_array_clear(&opts->haves_to_inspect);
 	oid_array_clear(&opts->shallows);
 }
 

index 0ccc47ee73..10629b2044 100644
@@ -667,6 +667,13 @@ struct odb_generate_pack_options {
 	 */
 	struct oid_array haves;
 
+	/*
+	 * Receiver-known commit tips whose root trees should be examined
+	 * to identify additional objects that need not be packed. This
+	 * optional inspection does not walk their commit histories.
+	 */
+	struct oid_array haves_to_inspect;
+
 	/*
 	 * The shallow boundary that shall be used when computing object
 	 * reachability. When set, any shallow information of the repository

@@ -751,6 +758,7 @@ struct odb_generate_pack_options {
 #define ODB_GENERATE_PACK_OPTIONS_INIT { \
 	.wants = OID_ARRAY_INIT, \
 	.haves = OID_ARRAY_INIT, \
+	.haves_to_inspect = OID_ARRAY_INIT, \
 	.shallows = OID_ARRAY_INIT, \
 	.pack_fd = -1, \
 }

index 455f409db0..bad9ecde4c 100644
@@ -1176,6 +1176,9 @@ static int odb_source_files_generate_pack(struct odb_source *source UNUSED,
 	fprintf(in, "--not\n");
 	for (size_t i = 0; i < opts->haves.nr; i++)
 		fprintf(in, "%s\n", oid_to_hex(&opts->haves.oid[i]));
+	for (size_t i = 0; i < opts->haves_to_inspect.nr; i++)
+		fprintf(in, "--inspect-have %s\n",
+			oid_to_hex(&opts->haves_to_inspect.oid[i]));
 	fprintf(in, "\n");
 	fflush(in);
 	if (ferror(in)) {

gitgitgadget

From: Marc Strapetz <[email protected]>

Rebasing a pushed branch can put its old tip outside the boundary traversed by pack-objects. Even with that tip among the negative revisions, unchanged blobs may then be sent again.

For forced non-fast-forward branch updates, pass locally available advertised old tips as haves_to_inspect. Limit this to refs selected for sending; leave fast-forward and rejected updates out. Restrict the hints to branches, which name commits, and skip shallow repositories, whose thin-pack pushes already inspect negative tips with --objects-edge-aggressive.

Update the rebase tests to expect two packed objects instead of
three: the rebased commit and its tree. The unchanged blob is already present on the receiver.

---
 send-pack.c           | 30 ++++++++++++++++++++++++------
 t/t5516-fetch-push.sh |  2 +-
 2 files changed, 25 insertions(+), 7 deletions(-)

index f20460fbf4..de6d33cf75 100644
@@ -42,17 +42,18 @@ int option_parse_push_signed(const struct option *opt,
 	die("bad %s argument: %s", opt->long_name, arg);
 }
 
-static void append_negative_object(struct repository *r,
-				   struct oid_array *haves,
-				   const struct object_id *oid)
+static int append_negative_object(struct repository *r,
+				  struct oid_array *haves,
+				  const struct object_id *oid)
 {
 	/*
 	 * The remote end may have advertised objects that we do not have in
 	 * our object database. Skip those, as we cannot use them as boundary.
 	 */
 	if (!odb_has_object(r->objects, oid, 0))
-		return;
+		return 0;
 	oid_array_append(haves, oid);
+	return 1;
 }
 
 /*

@@ -89,8 +90,25 @@ static int pack_objects(struct repository *r,
 		append_negative_object(r, &opts.haves, &negotiated->oid[i]);
 
 	while (refs) {
-		if (!is_null_oid(&refs->old_oid))
-			append_negative_object(r, &opts.haves, &refs->old_oid);
+		if (!is_null_oid(&refs->old_oid)) {
+			int added = append_negative_object(r, &opts.haves,
+							   &refs->old_oid);
+
+			/*
+			 * For forced pushes, inspect the old tip's tree to
+			 * avoid sending objects the receiver already has.
+			 * Shallow thin-pack pushes already inspect negative
+			 * tips via --objects-edge-aggressive. Limit this
+			 * optimization to branches because --inspect-have
+			 * requires commits, while other refs may legitimately
+			 * point to trees or blobs.
+			 */
+			if (added && refs->forced_update && !opts.shallow &&
+			    starts_with(refs->name, "refs/heads/") &&
+			    (refs->status == REF_STATUS_OK ||
+			     refs->status == REF_STATUS_EXPECTING_REPORT))
+				oid_array_append(&opts.haves_to_inspect, &refs->old_oid);
+		}
 		if (!is_null_oid(&refs->new_oid))
 			oid_array_append(&opts.wants, &refs->new_oid);
 		refs = refs->next;

index a46ba91ba0..067b4ddc16 100755
@@ -216,7 +216,7 @@ do
 			GIT_TRACE2_EVENT="$D/rebased-push-event" \
 			git -c push.useBitmaps=false push $push_args
 		) &&
-		grep_wrote 3 rebased-push-event # 1 commit, 1 tree, 1 blob
+		grep_wrote 2 rebased-push-event # 1 commit, 1 tree
 	'
 done
 

gitgitgadget

This sounds like the exact problem push negotiation was designed to address.

Thanks for pointing this out. This series aims to optimize Git's default packing for forced non-fast-forward updates, without adding a negotiation phase. I should have explained that scope more clearly in the cover letter:

pack-objects can resend unchanged objects from an advertised old branch tip even though send-pack already supplies that tip as an exclusion. This series improves default packing for forced updates by applying the existing sparse tree marking to selected old tips. It targets this inefficiency without the additional negotiation exchanges of push.negotiate or the broader recursive tree marking of --objects-edge-aggressive. Neither is enabled by default for ordinary pushes from non-shallow repositories. This series targets a narrower opportunity within that default behavior.

A divergent old tip can lie outside the boundary reached by the ordinary revision walk. Passing it to pack-objects as --inspect-have lets us compare its root tree with the outgoing trees, pruning identical and receiver-only subtrees. The additional inspection is limited to forced non-fast-forward updates of existing branches with locally available old tips. Large blobs retained across a rebase are a particularly costly example of the redundant transfer this avoids.

I would also add the following section at the end to explain how this relates to the existing optimizations:

push.negotiate discovers additional common commits through a separate fetch negotiation, potentially requiring multiple protocol round trips. This can reduce the pack when ref advertisement alone is insufficient, but adds latency even when the advertised tips already suffice. For small pushes that latency can outweigh the transfer savings. Even if negotiation achieves the same pack-size reduction, improving the default packing path avoids the need to pay that additional protocol cost.

--objects-edge-aggressive already considers excluded tips beyond the ordinary walk's boundary, but recursively marks their trees. The decision to enable it automatically only for shallow pushes was made for performance reasons: the 2014 series [1] records a regression from 4.785 to 23.322 seconds with 22,400 refs [2], and keeps non-shallow repositories on --objects-edge for better performance in the general case [3]. This series instead limits inspection to the old tips of branches being forcibly updated and uses sparse comparison to prune identical and receiver-only subtrees.

The additional inspection requires no protocol exchange or walk of the old tip's commit history. Ordinary fast-forward pushes keep their existing path, including those made with --force. No server or wire-protocol change is needed.

[1]
https://lore.kernel.org/git/[email protected]/T/

    [2]
https://github.com/git/git/commit/1684c1b219e02c91655ad929f752f4f864c72faf
    [3]

https://github.com/git/git/commit/2dacf26d0985521c0f30e535963a45257b63ea21

I'd like to give this series one more try, because I think the default push path still misses a worthwhile optimization opportunity.

Rebase-and-force-update workflows are common enough that avoiding redundant transfer on the default path appears worth evaluating. Junio pointed out that push negotiation addresses the same general problem. This series instead targets the non-negotiated default path: it uses the already advertised old destination tip, with no additional round trip, server change, or protocol change.

For qualifying non-fast-forward branch updates, this series can avoid resending objects already present at the destination. This can reduce pack size and network transfer, as well as client-side packing and server-side receive and indexing work. The scope is narrow: ordinary fast-forward pushes are unaffected, and the additional inspection is limited to forced updates with a locally available old tip.

The current measurements show both the potential and the tradeoff: the small cases improve, while the low-benefit worst-case fixture adds about 8% packing time for a negligible size reduction. These measurements cover only local pack generation; they do not include the primary expected benefits of a smaller pack: reduced network transfer and less receive and indexing work on the server. I would appreciate another look at whether this targeted optimization is worthwhile on the default path, and whether its current scope is narrow enough.

I've added Taylor and Derrick because of their work on pack generation and revision traversal. I'd be interested to hear whether this looks like a reasonable tradeoff for rewritten branch updates, or whether there is a simpler approach worth exploring.

-Marc