Skip to content

Commit 9a8c3c4

Browse files
peffgitster
authored andcommitted
parse_object(): check commit-graph when skip_hash set
If the caller told us that they don't care about us checking the object hash, then we're free to implement any optimizations that get us the parsed value more quickly. An obvious one is to check the commit graph before loading an object from disk. And in fact, both of the callers who pass in this flag are already doing so before they call parse_object()! So we can simplify those callers, as well as any possible future ones, by moving the logic into parse_object(). There are two subtle things to note in the diff, but neither has any impact in practice: - it seems least-surprising here to do the graph lookup on the git-replace'd oid, rather than the original. This is in theory a change of behavior from the earlier code, as neither caller did a replace lookup itself. But in practice it doesn't matter, as we disable the commit graph entirely if there are any replace refs. - the caller in get_reference() passes the skip_hash flag only if revs->verify_objects isn't set, whereas it would look in the commit graph unconditionally. In practice this should not matter as we should disable the commit graph entirely when using verify_objects (and that was done recently in another patch). So this should be a pure cleanup with no behavior change. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0bc2557 commit 9a8c3c4

File tree

3 files changed

+11
-20
lines changed

3 files changed

+11
-20
lines changed

object.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,12 @@ struct object *parse_object_with_flags(struct repository *r,
279279
if (obj && obj->parsed)
280280
return obj;
281281

282+
if (skip_hash) {
283+
struct commit *commit = lookup_commit_in_graph(r, repl);
284+
if (commit)
285+
return &commit->object;
286+
}
287+
282288
if ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||
283289
(!obj && repo_has_object_file(r, oid) &&
284290
oid_object_info(r, oid, NULL) == OBJ_BLOB)) {

revision.c

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -373,20 +373,10 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
373373
unsigned int flags)
374374
{
375375
struct object *object;
376-
struct commit *commit;
377376

378-
/*
379-
* If the repository has commit graphs, we try to opportunistically
380-
* look up the object ID in those graphs. Like this, we can avoid
381-
* parsing commit data from disk.
382-
*/
383-
commit = lookup_commit_in_graph(revs->repo, oid);
384-
if (commit)
385-
object = &commit->object;
386-
else
387-
object = parse_object_with_flags(revs->repo, oid,
388-
revs->verify_objects ? 0 :
389-
PARSE_OBJECT_SKIP_HASH_CHECK);
377+
object = parse_object_with_flags(revs->repo, oid,
378+
revs->verify_objects ? 0 :
379+
PARSE_OBJECT_SKIP_HASH_CHECK);
390380

391381
if (!object) {
392382
if (revs->ignore_missing)

upload-pack.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,19 +1409,14 @@ static int parse_want(struct packet_writer *writer, const char *line,
14091409
const char *arg;
14101410
if (skip_prefix(line, "want ", &arg)) {
14111411
struct object_id oid;
1412-
struct commit *commit;
14131412
struct object *o;
14141413

14151414
if (get_oid_hex(arg, &oid))
14161415
die("git upload-pack: protocol error, "
14171416
"expected to get oid, not '%s'", line);
14181417

1419-
commit = lookup_commit_in_graph(the_repository, &oid);
1420-
if (commit)
1421-
o = &commit->object;
1422-
else
1423-
o = parse_object_with_flags(the_repository, &oid,
1424-
PARSE_OBJECT_SKIP_HASH_CHECK);
1418+
o = parse_object_with_flags(the_repository, &oid,
1419+
PARSE_OBJECT_SKIP_HASH_CHECK);
14251420

14261421
if (!o) {
14271422
packet_writer_error(writer,

0 commit comments

Comments
 (0)