Re: [PATCH v2 11/11] reftable/block: reuse buffer to compute record keys

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 28, 2023, 05:53 UTC
Message-ID: <ZY0Ndkv2v7TXVskc@tanuki>
In-Reply-To: <CAOw_e7bQ+jxO2zhj32mDksq9uBKQfNt=wMNP5K6Oy1DqievCdg@mail.gmail.com>

On Thu, Dec 21, 2023 at 11:43:27AM +0100, Han-Wen Nienhuys wrote:

Show 18 quoted lines

> On Fri, Dec 8, 2023 at 3:53 PM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > @@ -84,10 +84,12 @@ struct block_iter {
> >
> >         /* key for last entry we read. */
> >         struct strbuf last_key;
> > +       struct strbuf key;
> >  };
> 
> it's slightly more efficient, but the new field has no essential
> meaning. If I encountered this code with the change you make here, I
> would probably refactor it in the opposite direction to increase code
> clarity.
> 
> I suspect that the gains are too small to be measurable, but if you
> are after small efficiency gains, you can have
> reftable_record_decode() consume the key to avoid copying overhead in
> record.c.

Fair enough. I've done a better job for these kinds of refactorings for the reftable library in my second patch series [1] by including some benchmarks via Valgrind. This should result in less handwavy and actually measurable/significant performance improvements.

Patrick

[1]: https://public-inbox.org/git/xmqqr0jgsn9g.fsf@gitster.g/T/#t