[PATCH v4 6/6] revision: avoid hitting packfiles when commits are in commit-graph

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 4757 bytes --]

When queueing references in git-rev-list(1), we try to either reuse an
already parsed object or alternatively we load the object header from
disk in order to determine its type. This is inefficient for commits
though in cases where we have a commit graph available: instead of
hitting the real object on disk to determine its type, we may instead
search the object graph for the object ID. In case it's found, we can
directly fill in the commit object, otherwise we can still hit the disk
to determine the object's type.

Expose a new function `parse_commit_in_graph_gently()`, which fills in
an object of unknown type in case we find its object ID in the graph.
This provides a big performance win in cases where there is a
commit-graph available in the repository in case we load lots of
references. The following has been executed in a real-world repository
with about 2.2 million refs:

    Benchmark #1: HEAD~: rev-list --unsorted-input --objects --quiet --not --all --not $newrev
      Time (mean ± σ):      4.508 s ±  0.039 s    [User: 4.131 s, System: 0.377 s]
      Range (min … max):    4.455 s …  4.576 s    10 runs

    Benchmark #2: HEAD: rev-list --unsorted-input --objects --quiet --not --all --not $newrev
      Time (mean ± σ):      3.072 s ±  0.031 s    [User: 2.707 s, System: 0.365 s]
      Range (min … max):    3.040 s …  3.144 s    10 runs

    Summary
      'HEAD: rev-list --unsorted-input --objects --quiet --not --all --not $newrev' ran
        1.47 ± 0.02 times faster than 'HEAD~: rev-list --unsorted-input --objects --quiet --not --all --not $newrev'

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 commit-graph.c | 26 ++++++++++++++++++++++++++
 commit-graph.h |  7 +++++++
 revision.c     | 17 +++++++++++++++--
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index 8c4c7262c8..cc7136243d 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -891,6 +891,32 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g,
 	}
 }
 
+int parse_commit_in_graph_gently(struct repository *repo, struct object *object)
+{
+	struct commit *commit;
+	uint32_t pos;
+
+	if (object->parsed) {
+		if (object->type != OBJ_COMMIT)
+			return -1;
+		return 0;
+	}
+
+	if (!repo->objects->commit_graph)
+		return -1;
+
+	if (!search_commit_pos_in_graph(&object->oid, repo->objects->commit_graph, &pos))
+		return -1;
+
+	commit = object_as_type(object, OBJ_COMMIT, 1);
+	if (!commit)
+		return -1;
+	if (!fill_commit_in_graph(repo, commit, repo->objects->commit_graph, pos))
+		return -1;
+
+	return 0;
+}
+
 static int parse_commit_in_graph_one(struct repository *r,
 				     struct commit_graph *g,
 				     struct commit *item)
diff --git a/commit-graph.h b/commit-graph.h
index 96c24fb577..e2e93b0ee1 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -40,6 +40,13 @@ int open_commit_graph(const char *graph_file, int *fd, struct stat *st);
  */
 int parse_commit_in_graph(struct repository *r, struct commit *item);
 
+/*
+ * Given an object of unknown type, try to fill in the object in case it is a
+ * commit part of the commit-graph. Returns 0 if the object is a parsed commit
+ * or if it could be filled in via the commit graph, otherwise it returns -1.
+ */
+int parse_commit_in_graph_gently(struct repository *repo, struct object *object);
+
 /*
  * It is possible that we loaded commit contents from the commit buffer,
  * but we also want to ensure the commit-graph content is correctly
diff --git a/revision.c b/revision.c
index 25f4784fdd..318b43026e 100644
--- a/revision.c
+++ b/revision.c
@@ -362,8 +362,21 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
 	struct object *object = lookup_unknown_object(revs->repo, oid);
 
 	if (object->type == OBJ_NONE) {
-		int type = oid_object_info(revs->repo, oid, NULL);
-		if (type < 0 || !object_as_type(object, type, 1)) {
+		/*
+		 * It's likely that the reference points to a commit, so we
+		 * first try to look it up via the commit-graph. If successful,
+		 * then we know it's a commit and don't have to unpack the
+		 * object header. We still need to assert that the object
+		 * exists, but given that we don't request any info about the
+		 * object this is a lot faster than `oid_object_info()`.
+		 */
+		if (parse_commit_in_graph_gently(revs->repo, object) < 0) {
+			int type = oid_object_info(revs->repo, oid, NULL);
+			if (type < 0 || !object_as_type(object, type, 1)) {
+				object = NULL;
+				goto out;
+			}
+		} else if (!repo_has_object_file(revs->repo, oid)) {
 			object = NULL;
 			goto out;
 		}
-- 
2.32.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]