Thread (13 messages) 13 messages, 6 authors, 2023-11-11

Re: [RFC PATCH 2/3] tmp-objdir: introduce `tmp_objdir_repack()`

From: Patrick Steinhardt <hidden>
Date: 2023-11-08 07:05:55

On Tue, Nov 07, 2023 at 01:22:58PM -0500, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
In the following commit, we will teach `git replay` how to write a pack
containing the set of new objects created as a result of the `replay`
operation.

Since `replay` needs to be able to see the object(s) written
from previous steps in order to replay each commit, the ODB transaction
may have multiple pending packs. Migrating multiple packs back into the
main object store has a couple of downsides:

  - It is error-prone to do so: each pack must be migrated in the
    correct order (with the ".idx" file staged last), and the set of
    packs themselves must be moved over in the correct order to avoid
    racy behavior.

  - It is a (potentially significant) performance degradation to migrate
    a large number of packs back into the main object store.

Introduce a new function that combines the set of all packs in the
temporary object store to produce a single pack which is the logical
concatenation of all packs created during that level of the ODB
transaction.

Signed-off-by: Taylor Blau <redacted>
---
 tmp-objdir.c | 13 +++++++++++++
 tmp-objdir.h |  6 ++++++
 2 files changed, 19 insertions(+)
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 5f9074ad1c..ef53180b47 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -12,6 +12,7 @@
 #include "strvec.h"
 #include "quote.h"
 #include "object-store-ll.h"
+#include "run-command.h"
 
 struct tmp_objdir {
 	struct strbuf path;
@@ -277,6 +278,18 @@ int tmp_objdir_migrate(struct tmp_objdir *t)
 	return ret;
 }
 
+int tmp_objdir_repack(struct tmp_objdir *t)
+{
+	struct child_process cmd = CHILD_PROCESS_INIT;
+
+	cmd.git_cmd = 1;
+
+	strvec_pushl(&cmd.args, "repack", "-a", "-d", "-k", "-l", NULL);
+	strvec_pushv(&cmd.env, tmp_objdir_env(t));
I wonder what performance of this repack would be like in a large
repository with many refs. Ideally, I would expect that the repacking
performance should scale with the number of objects we have written into
the temporary object directory. But in practice, the repack will need to
compute reachability and thus also scales with the size of the repo
itself, doesn't it?

Patrick
quoted hunk ↗ jump to hunk
+	return run_command(&cmd);
+}
+
 const char **tmp_objdir_env(const struct tmp_objdir *t)
 {
 	if (!t)
diff --git a/tmp-objdir.h b/tmp-objdir.h
index 237d96b660..d00e3b3e27 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -36,6 +36,12 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix);
  */
 const char **tmp_objdir_env(const struct tmp_objdir *);
 
+/*
+ * Combines all packs in the tmp_objdir into a single pack before migrating.
+ * Removes original pack(s) after installing the combined pack into place.
+ */
+int tmp_objdir_repack(struct tmp_objdir *);
+
 /*
  * Finalize a temporary object directory by migrating its objects into the main
  * object database, removing the temporary directory, and freeing any
-- 
2.42.0.446.g0b9ef90488

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help