git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: peff@peff.net
Subject: [PATCH 3/3] builtin/pack-objects.c: remove duplicate hash lookup
Date: Sun, 29 Aug 2021 22:48:57 -0400	[thread overview]
Message-ID: <5f7f88ac1dc022de2ae5108828a1f61f7150686b.1630291682.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1630291682.git.me@ttaylorr.com>

In the original code from 08cdfb1337 (pack-objects --keep-unreachable,
2007-09-16), we add each object to the packing list with type
`obj->type`, where `obj` comes from `lookup_unknown_object()`. Unless we
had already looked up and parsed the object, this will be `OBJ_NONE`.
That's fine, since oe_set_type() sets the type_valid bit to '0', and we
determine the real type later on.

So the only thing we need from the object lookup is access to the
`flags` field so that we can mark that we've added the object with
`OBJECT_ADDED` to avoid adding it again (we can just pass `OBJ_NONE`
directly instead of grabbing it from the object).

But add_object_entry() already rejects duplicates! This has been the
behavior since 7a979d99ba (Thin pack - create packfile with missing
delta base., 2006-02-19), but 08cdfb1337 didn't take advantage of it.
Moreover, to do the OBJECT_ADDED check, we have to do a hash lookup in
`obj_hash`.

So we can drop the lookup_unknown_object() call completely, *and* the
OBJECT_ADDED flag, too, since the spot we're touching here is the only
location that checks it.

In the end, we perform the same number of hash lookups, but with the
added bonus that we don't waste memory allocating an OBJ_NONE object (if
we were traversing, we'd need it eventually, but the whole point of this
code path is not to traverse).

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/pack-objects.c | 11 +----------
 object.h               |  1 -
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 87ddbd5553..ec8503563a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3405,13 +3405,9 @@ static void read_object_list_from_stdin(void)
 	}
 }
 
-/* Remember to update object flag allocation in object.h */
-#define OBJECT_ADDED (1u<<20)
-
 static void show_commit(struct commit *commit, void *data)
 {
 	add_object_entry(&commit->object.oid, OBJ_COMMIT, NULL, 0);
-	commit->object.flags |= OBJECT_ADDED;
 
 	if (write_bitmap_index)
 		index_commit_for_bitmap(commit);
@@ -3424,7 +3420,6 @@ static void show_object(struct object *obj, const char *name, void *data)
 {
 	add_preferred_base_object(name);
 	add_object_entry(&obj->oid, obj->type, name, 0);
-	obj->flags |= OBJECT_ADDED;
 
 	if (use_delta_islands) {
 		const char *p;
@@ -3510,11 +3505,7 @@ static int add_object_in_unpacked_pack(const struct object_id *oid,
 				       uint32_t pos,
 				       void *_data)
 {
-	struct object *obj = lookup_unknown_object(the_repository, oid);
-	if (obj->flags & OBJECT_ADDED)
-		return 0;
-	add_object_entry(oid, obj->type, "", 0);
-	obj->flags |= OBJECT_ADDED;
+	add_object_entry(oid, OBJ_NONE, "", 0);
 	return 0;
 }
 
diff --git a/object.h b/object.h
index 3b38c9cc98..549f2d256b 100644
--- a/object.h
+++ b/object.h
@@ -75,7 +75,6 @@ struct object_array {
  * builtin/fsck.c:           0--3
  * builtin/gc.c:             0
  * builtin/index-pack.c:                                     2021
- * builtin/pack-objects.c:                                   20
  * builtin/reflog.c:                   10--12
  * builtin/show-branch.c:    0-------------------------------------------26
  * builtin/unpack-objects.c:                                 2021
-- 
2.33.0.96.g73915697e6

  parent reply	other threads:[~2021-08-30  2:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30  2:48 [PATCH 0/3] pack-objects: simplify add_objects_in_unpacked_packs() Taylor Blau
2021-08-30  2:48 ` [PATCH 1/3] object-store.h: teach for_each_packed_object to ignore kept packs Taylor Blau
2021-08-30  2:48 ` [PATCH 2/3] builtin/pack-objects.c: simplify add_objects_in_unpacked_packs() Taylor Blau
2021-09-02 23:04   ` Jeff King
2021-09-03  2:33     ` Taylor Blau
2021-08-30  2:48 ` Taylor Blau [this message]
2021-08-30  6:39 ` [PATCH 0/3] pack-objects: " Junio C Hamano
2021-08-30 20:58 ` Jeff King
2021-08-30 21:30   ` Taylor Blau

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5f7f88ac1dc022de2ae5108828a1f61f7150686b.1630291682.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).