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

From: Taylor Blau <me@ttaylorr.com>
Date: Dec 8, 2023, 22:17 UTC
Message-ID: <ZXOV8TCqaH0xXRnS@nand.local>
In-Reply-To: <f797feff8dec383f1db9ae403cd89b80d1743432.1702047081.git.ps@pks.im>

On Fri, Dec 08, 2023 at 03:53:18PM +0100, Patrick Steinhardt wrote:

Show 9 quoted lines

> 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.

Show 7 quoted lines

> @@ -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