@matheusbsbs / matheus-bagliano / commits / 1687150b5d

hashfile: allow skipping the hash function

The hashfile API is useful for generating files that include a trailing hash of the file's contents up to that point. Using such a hash is helpful for verifying the file for corruption-at-rest, such as a faulty drive causing flipped bits. Git's index file includes this trailing hash, so it uses a 'struct hashfile' to handle the I/O to the file. This was very convenient to allow using the hashfile methods during these operations. However, hashing the file contents during write comes at a performance penalty. It's slower to hash the bytes on their way to the disk than without that step. This problem is made worse by the replacement of hardware-accelerated SHA1 computations with the software-based sha1dc computation. This write cost is significant, and the checksum capability is likely not worth that cost for such a short-lived file. The index is rewritten frequently and the only time the checksum is checked is during 'git fsck'. Thus, it would be helpful to allow a user to opt-out of the hash computation. We first need to allow Git to opt-out of the hash computation in the hashfile API. The buffered writes of the API are still helpful, so it makes sense to make the change here. Introduce a new 'skip_hash' option to 'struct hashfile'. When set, the update_fn and final_fn members of the_hash_algo are skipped. When finalizing the hashfile, the trailing hash is replaced with the null hash. This use of a trailing null hash would be desireable in either case, since we do not want to special case a file format to have a different length depending on whether it was hashed or not. When the final bytes of a file are all zero, we can infer that it was written without hashing, and thus that verification is not available as a check for file consistency. This also means that we could easily toggle hashing for any file format we desire. A version of this patch has existed in the microsoft/git fork since 2017 [1] (the linked commit was rebased in 2018, but the original dates back to January 2017). Here, the change to make the index use this fast path is delayed until a later change. [1] https://github.com/microsoft/git/commit/21fed2d91410f45d85279467f21d717a2db45201 Co-authored-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Jan 6, 2023 at 16:31 UTC 1687150b5dbb570746d7f31537e199210d938091

2 files changed +18 -3

csum-file.c

+11 -3

@@ -45,7 +45,8 @@ void hashflush(struct hashfile *f)

45 unsigned offset = f->offset;

47 if (offset) {

48 - the_hash_algo->update_fn(&f->ctx, f->buffer, offset);

48 + if (!f->skip_hash)

49 + the_hash_algo->update_fn(&f->ctx, f->buffer, offset);

50 flush(f, f->buffer, offset);

51 f->offset = 0;

52 }

@@ -64,7 +65,12 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result,

65 int fd;

67 hashflush(f);

67 - the_hash_algo->final_fn(f->buffer, &f->ctx);

68 +

69 + if (f->skip_hash)

70 + hashclr(f->buffer);

71 + else

72 + the_hash_algo->final_fn(f->buffer, &f->ctx);

73 +

74 if (result)

75 hashcpy(result, f->buffer);

76 if (flags & CSUM_HASH_IN_STREAM)

@@ -108,7 +114,8 @@ void hashwrite(struct hashfile *f, const void *buf, unsigned int count)

114 * the hashfile's buffer. In this block,

115 * f->offset is necessarily zero.

116 */

111 - the_hash_algo->update_fn(&f->ctx, buf, nr);

117 + if (!f->skip_hash)

118 + the_hash_algo->update_fn(&f->ctx, buf, nr);

119 flush(f, buf, nr);

120 } else {

121 /*

@@ -153,6 +160,7 @@ static struct hashfile *hashfd_internal(int fd, const char *name,

160 f->tp = tp;

161 f->name = name;

162 f->do_crc = 0;

163 + f->skip_hash = 0;

164 the_hash_algo->init_fn(&f->ctx);

165

166 f->buffer_len = buffer_len;

csum-file.h

@@ -20,6 +20,13 @@ struct hashfile {

20 size_t buffer_len;

21 unsigned char *buffer;

22 unsigned char *check_buffer;

23 +

24 + /**

25 + * If non-zero, skip_hash indicates that we should

26 + * not actually compute the hash for this hashfile and

27 + * instead only use it as a buffered write.

28 + */

29 + int skip_hash;

30 };

32 /* Checkpoint */