git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 3/3] raw_object_store: drop extra pointer to replace_map
Date: Mon, 12 May 2025 14:52:33 -0400	[thread overview]
Message-ID: <20250512185233.GC1276214@coredump.intra.peff.net> (raw)
In-Reply-To: <20250512185006.GA1275914@coredump.intra.peff.net>

We store the replacement data in an oidmap, which is itself a pointer in
the raw_object_store struct. But there's no need for an extra pointer
indirection here. It is always allocated and initialized along with the
containing struct, and we never check it for NULL-ness.

Let's embed the map directly in the struct, which is simpler and avoids
extra pointer chasing.

Signed-off-by: Jeff King <peff@peff.net>
---
This one may be more subjective, but IMHO it's good to avoid extra
pointers when we can.

 commit-graph.c   | 2 +-
 object-store.c   | 3 +--
 object-store.h   | 3 ++-
 replace-object.c | 8 +++-----
 replace-object.h | 2 +-
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index 1a74e1e1ba..4a6e34f8a0 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -222,7 +222,7 @@ static int commit_graph_compatible(struct repository *r)
 
 	if (replace_refs_enabled(r)) {
 		prepare_replace_object(r);
-		if (oidmap_get_size(r->objects->replace_map))
+		if (oidmap_get_size(&r->objects->replace_map))
 			return 0;
 	}
 
diff --git a/object-store.c b/object-store.c
index bc24e80829..911bc7ff5f 100644
--- a/object-store.c
+++ b/object-store.c
@@ -1017,8 +1017,7 @@ void raw_object_store_clear(struct raw_object_store *o)
 {
 	FREE_AND_NULL(o->alternate_db);
 
-	oidmap_clear(o->replace_map, 1);
-	FREE_AND_NULL(o->replace_map);
+	oidmap_clear(&o->replace_map, 1);
 	pthread_mutex_destroy(&o->replace_mutex);
 
 	free_commit_graph(o->commit_graph);
diff --git a/object-store.h b/object-store.h
index 46961dc954..9f6f27c016 100644
--- a/object-store.h
+++ b/object-store.h
@@ -5,6 +5,7 @@
 #include "object.h"
 #include "list.h"
 #include "oidset.h"
+#include "oidmap.h"
 #include "thread-utils.h"
 
 struct oidmap;
@@ -176,7 +177,7 @@ struct raw_object_store {
 	 * Objects that should be substituted by other objects
 	 * (see git-replace(1)).
 	 */
-	struct oidmap *replace_map;
+	struct oidmap replace_map;
 	unsigned replace_map_initialized : 1;
 	pthread_mutex_t replace_mutex; /* protect object replace functions */
 
diff --git a/replace-object.c b/replace-object.c
index 7b8a09b5cb..f8c5f68837 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -31,7 +31,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (oidmap_put(r->objects->replace_map, repl_obj))
+	if (oidmap_put(&r->objects->replace_map, repl_obj))
 		die(_("duplicate replace ref: %s"), refname);
 
 	return 0;
@@ -48,9 +48,7 @@ void prepare_replace_object(struct repository *r)
 		return;
 	}
 
-	r->objects->replace_map =
-		xmalloc(sizeof(*r->objects->replace_map));
-	oidmap_init(r->objects->replace_map, 0);
+	oidmap_init(&r->objects->replace_map, 0);
 
 	refs_for_each_replace_ref(get_main_ref_store(r),
 				  register_replace_ref, r);
@@ -80,7 +78,7 @@ const struct object_id *do_lookup_replace_object(struct repository *r,
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
 		struct replace_object *repl_obj =
-			oidmap_get(r->objects->replace_map, cur);
+			oidmap_get(&r->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
diff --git a/replace-object.h b/replace-object.h
index 4226376534..3052e96a62 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -47,7 +47,7 @@ static inline const struct object_id *lookup_replace_object(struct repository *r
 {
 	if (!replace_refs_enabled(r) ||
 	    (r->objects->replace_map_initialized &&
-	     oidmap_get_size(r->objects->replace_map) == 0))
+	     oidmap_get_size(&r->objects->replace_map) == 0))
 		return oid;
 	return do_lookup_replace_object(r, oid);
 }
-- 
2.49.0.821.gd3b3298025

  parent reply	other threads:[~2025-05-12 18:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-12 18:50 [PATCH 0/3] minor oidmap cleanups Jeff King
2025-05-12 18:50 ` [PATCH 1/3] oidmap: rename oidmap_free() to oidmap_clear() Jeff King
2025-05-12 18:51 ` [PATCH 2/3] oidmap: add size function Jeff King
2025-05-13  9:48   ` Patrick Steinhardt
2025-05-14 17:59     ` Jeff King
2025-05-12 18:52 ` Jeff King [this message]
2025-05-13  9:48   ` [PATCH 3/3] raw_object_store: drop extra pointer to replace_map Patrick Steinhardt
2025-05-14 14:38     ` Junio C Hamano

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=20250512185233.GC1276214@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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 NNTP newsgroup(s).