Thread (37 messages) 37 messages, 3 authors, 2025-07-15

Re: [PATCH 1/8] midx: start tracking per object database source

From: Patrick Steinhardt <hidden>
Date: 2025-07-15 08:26:37

On Thu, Jul 10, 2025 at 07:10:02PM -0400, Taylor Blau wrote:
On Wed, Jul 09, 2025 at 09:54:49AM +0200, Patrick Steinhardt wrote:
quoted
Multi-pack indices are tracked via `struct multi_pack_index`. This data
structure is stored as a linked list inside `struct object_database`,
which is the global database that spans across all of the object
sources.

This layout causes two problems:

  - Multi-pack indices aren't global to an object database, but instead
    there can be one multi-pack index per source. This creates a
    mismatch between the on-disk layout and how things are organized in
    the object database subsystems and makes some parts, like figuring
    out whether a source has an MIDX, quite awkward.
This is a little confusing to me. What do we consider to be an "object
database", and what do we consider to be a "source"?

For instance, if I have a repository with one or more alternates, I
would imagine that each alternate is a separate "source", and the
sources together comprise the object database. Does that match the way
you're thinking about it?

If so, that makes sense. But if not (i.e., we consider all alternates to
belong to the same object database and share a single source), then I
don't know how this will interact with the existing MIDX alternates
mechanism.

Some clarification here would be helpful, I think.
I'll add some, sure. This whole infra is currently developing, so it
doesn't hurt to reiterate some of the points that have already landed.
quoted
diff --git a/midx.c b/midx.c
index 3c5bc821730..a91231bfcdf 100644
--- a/midx.c
+++ b/midx.c
@@ -724,28 +724,29 @@ int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id)
 	return 0;
 }

-int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local)
+int prepare_multi_pack_index_one(struct odb_source *source, int local)
OK, so the combination of a repository and "object_dir" path becomes a
new "struct odb_source", which makes sense. But shouldn't "local" also
be a property of the odb_source?

I am not familiar enough with the odb_source work thus far to know
whether that is a good idea or not, but just something that stood out
to me while reading.
Yes, you're right on point. I have a follow-up patch series that'll
eventually get rid of some duplicate information that is now present in
the ODB source connected to an MIDX.

[snip]
quoted
+		source->multi_pack_index = m;
As an aside, I see that we're calling this new field "multi_pack_index".
I wonder if it would be better to call it "midx", since typing out
"multi_pack_index" is a little verbose, and "midx" is a common
abbreviation that I don't think we would cause any confusion with.
Sure, I can do that.
quoted
diff --git a/midx.h b/midx.h
index 9d1374cbd58..b1626a9a7c4 100644
--- a/midx.h
+++ b/midx.h
@@ -3,11 +3,12 @@

 #include "string-list.h"

+struct bitmapped_pack;
+struct git_hash_algo;
 struct object_id;
+struct odb_source;
 struct pack_entry;
 struct repository;
-struct bitmapped_pack;
-struct git_hash_algo;
(I'm nit-picking, but a similar note here that the unrelated changes
make it harder to see what is actually going on in this hunk, which is
adding a declaration for "struct odb_source".)
quoted
diff --git a/odb.h b/odb.h
index e922f256802..8e79c7be520 100644
--- a/odb.h
+++ b/odb.h
@@ -9,10 +
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help