parse_object(): allow skipping hash check

The parse_object() function checks the object hash of any object it parses. This is a nice feature, as it means we may catch bit corruption during normal use, rather than waiting for specific fsck operations. But it also can be slow. It's particularly noticeable for blobs, where except for the hash check, we could return without loading the object contents at all. Now one may wonder what is the point of calling parse_object() on a blob in the first place then, but usually it's not intentional: we were fed an oid from somewhere, don't know the type, and want an object struct. For commits and trees, the parsing is usually helpful; we're about to look at the contents anyway. But this is less true for blobs, where we may be collecting them as part of a reachability traversal, etc, and don't actually care what's in them. And blobs, of course, tend to be larger. We don't want to just throw out the hash-checks for blobs, though. We do depend on them in some circumstances (e.g., rev-list --verify-objects uses parse_object() to check them). It's only the callers that know how they're going to use the result. And so we can help them by providing a special flag to skip the hash check. We could just apply this to blobs, as they're going to be the main source of performance improvement. But if a caller doesn't care about checking the hash, we might as well skip it for other object types, too. Even though we can't avoid reading the object contents, we can still skip the actual hash computation. If this seems like it is making Git a little bit less safe against corruption, it may be. But it's part of a series of tradeoffs we're already making. For instance, "rev-list --objects" does not open the contents of blobs it prints. And when a commit graph is present, we skip opening most commits entirely. The important thing will be to use this flag in cases where it's safe to skip the check. For instance, when serving a pack for a fetch, we know the client will fully index the objects and do a connectivity check itself. There's little to be gained from the server side re-hashing a blob itself. And indeed, most of the time we don't! The revision machinery won't open up a blob reached by traversal, but only one requested directly with a "want" line. So applied properly, this new feature shouldn't make anything less safe in practice. 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:01 UTC c868d8e91fee01999eb34c5559250ea059293b33
2 files changed +18 -3
object.c
+12 -3
@@ -263,8 +263,11 @@ struct object *parse_object_or_die(const struct object_id *oid,
263 die(_("unable to parse object: %s"), name ? name : oid_to_hex(oid));
264 }
265
266 -struct object *parse_object(struct repository *r, const struct object_id *oid)
266 +struct object *parse_object_with_flags(struct repository *r,
267 + const struct object_id *oid,
268 + enum parse_object_flags flags)
269 {
270 + int skip_hash = !!(flags & PARSE_OBJECT_SKIP_HASH_CHECK);
271 unsigned long size;
272 enum object_type type;
273 int eaten;
@@ -279,7 +282,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
282 if ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||
283 (!obj && repo_has_object_file(r, oid) &&
284 oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
282 - if (stream_object_signature(r, repl) < 0) {
285 + if (!skip_hash && stream_object_signature(r, repl) < 0) {
286 error(_("hash mismatch %s"), oid_to_hex(oid));
287 return NULL;
288 }
@@ -289,7 +292,8 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
292
293 buffer = repo_read_object_file(r, oid, &type, &size);
294 if (buffer) {
292 - if (check_object_signature(r, repl, buffer, size, type) < 0) {
295 + if (!skip_hash &&
296 + check_object_signature(r, repl, buffer, size, type) < 0) {
297 free(buffer);
298 error(_("hash mismatch %s"), oid_to_hex(repl));
299 return NULL;
@@ -304,6 +308,11 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
308 return NULL;
309 }
310
311 +struct object *parse_object(struct repository *r, const struct object_id *oid)
312 +{
313 + return parse_object_with_flags(r, oid, 0);
314 +}
315 +
316 struct object_list *object_list_insert(struct object *item,
317 struct object_list **list_p)
318 {
object.h
+6
@@ -128,7 +128,13 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet);
128 *
129 * Returns NULL if the object is missing or corrupt.
130 */
131 +enum parse_object_flags {
132 + PARSE_OBJECT_SKIP_HASH_CHECK = 1 << 0,
133 +};
134 struct object *parse_object(struct repository *r, const struct object_id *oid);
135 +struct object *parse_object_with_flags(struct repository *r,
136 + const struct object_id *oid,
137 + enum parse_object_flags flags);
138
139 /*
140 * Like parse_object, but will die() instead of returning NULL. If the