On 2024.03.05 13:11, Patrick Steinhardt wrote:
> When decoding log records we need a temporary buffer to decode the
> reflog entry's name, mail address and message. As this buffer is local
> to the function we thus have to reallocate it for every single log
> record which we're about to decode, which is inefficient.
>
> Refactor the code such that callers need to pass in a scratch buffer,
> which allows us to reuse it for multiple decodes. This reduces the
> number of allocations when iterating through reflogs. Before:
>
> HEAP SUMMARY:
> in use at exit: 13,473 bytes in 122 blocks
> total heap usage: 2,068,487 allocs, 2,068,365 frees, 305,122,946 bytes allocated
>
> After:
>
> HEAP SUMMARY:
> in use at exit: 13,473 bytes in 122 blocks
> total heap usage: 1,068,485 allocs, 1,068,363 frees, 281,122,886 bytes allocated
>
> Note that this commit also drop some redundant calls to `strbuf_reset()`
> right before calling `decode_string()`. The latter already knows to
> reset the buffer, so there is no need for these.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> reftable/block.c | 4 ++-
> reftable/block.h | 2 ++
> reftable/record.c | 52 ++++++++++++++++++--------------------
> reftable/record.h | 5 ++--
> reftable/record_test.c | 57 ++++++++++++++++++++++++++----------------
> 5 files changed, 68 insertions(+), 52 deletions(-)
[snip]
> diff --git a/reftable/record.c b/reftable/record.c
> index 7c86877586..060244337f 100644
> --- a/reftable/record.c
> +++ b/reftable/record.c
> @@ -374,7 +374,7 @@ static int reftable_ref_record_encode(const void *rec, struct string_view s,
>
> static int reftable_ref_record_decode(void *rec, struct strbuf key,
> uint8_t val_type, struct string_view in,
> - int hash_size)
> + int hash_size, struct strbuf *scratch)
> {
> struct reftable_ref_record *r = rec;
> struct string_view start = in;
> @@ -425,13 +425,12 @@ static int reftable_ref_record_decode(void *rec, struct strbuf key,
> break;
>
> case REFTABLE_REF_SYMREF: {
> - struct strbuf dest = STRBUF_INIT;
> - int n = decode_string(&dest, in);
> + int n = decode_string(scratch, in);
> if (n < 0) {
> return -1;
> }
> string_view_consume(&in, n);
> - r->value.symref = dest.buf;
> + r->value.symref = strbuf_detach(scratch, NULL);
> } break;
I had to dig into this to convince myself that we aren't leaking memory
here, but IIUC this gets cleaned up eventually by
reftable_ref_record_release(), right?
