Subject: Re: [PATCH v2 06/11] reftable/stack: reuse buffers when reloading stack

From: Taylor Blau <[email-masked]>
Date: Fri, 8 Dec 2023 17:17:21 -0500

On Fri, Dec 08, 2023 at 03:53:18PM +0100, Patrick Steinhardt wrote:
> In `reftable_stack_reload_once()` we iterate over all the tables added
> to the stack in order to figure out whether any of the tables needs to
> be reloaded. We use a set of buffers in this context to compute the
> paths of these tables, but discard those buffers on every iteration.
> This is quite wasteful given that we do not need to transfer ownership
> of the allocated buffer outside of the loop.
>
> Refactor the code to instead reuse the buffers to reduce the number of
> allocations we need to do.

> @@ -267,16 +265,13 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
>  	for (i = 0; i < cur_len; i++) {
>  		if (cur[i]) {
>  			const char *name = reader_name(cur[i]);
> -			struct strbuf filename = STRBUF_INIT;
> -			stack_filename(&filename, st, name);
> +			stack_filename(&table_path, st, name);

This initially caught me by surprise, but on closer inspection I agree
that this is OK, since stack_filename() calls strbuf_reset() before
adjusting the buffer contents.

(As a side-note, I do find the side-effect of stack_filename() to be a
little surprising, but that's not the fault of this series and not worth
changing here.)

Thanks,
Taylor