@matheusbsbs / matheus-bagliano / commits / 9a8c3c4a5f

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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 6, 2022 at 19:06 UTC 9a8c3c4a5f94aaed54ae26e4796c08ca9a1cbfbc

3 files changed +11 -20

object.c

@@ -279,6 +279,12 @@ struct object *parse_object_with_flags(struct repository *r,

279 if (obj && obj->parsed)

280 return obj;

281

282 + if (skip_hash) {

283 + struct commit *commit = lookup_commit_in_graph(r, repl);

284 + if (commit)

285 + return &commit->object;

286 + }

287 +

288 if ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||

289 (!obj && repo_has_object_file(r, oid) &&

290 oid_object_info(r, oid, NULL) == OBJ_BLOB)) {

revision.c

+3 -13

@@ -373,20 +373,10 @@ static struct object *get_reference(struct rev_info *revs, const char *name,

373 unsigned int flags)

374 {

375 struct object *object;

376 - struct commit *commit;

376

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);

380

381 if (!object) {

382 if (revs->ignore_missing)

upload-pack.c

+2 -7

@@ -1409,19 +1409,14 @@ static int parse_want(struct packet_writer *writer, const char *line,

1409 const char *arg;

1410 if (skip_prefix(line, "want ", &arg)) {

1411 struct object_id oid;

1412 - struct commit *commit;

1412 struct object *o;

1413

1414 if (get_oid_hex(arg, &oid))

1415 die("git upload-pack: protocol error, "

1416 "expected to get oid, not '%s'", line);

1417

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);

1420

1421 if (!o) {

1422 packet_writer_error(writer,