git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 6/7] reftable/record: use scratch buffer when decoding records

From
Josh Steadmon <steadmon@google.com>
Date
Mar 11, 2024, 19:31 UTC
Message-ID
<Ze9cLWCZoBqcfr2B@google.com>
In-Reply-To
<e0a9057593f761917a3e6fb9a77045400e2b9d1c.1709640322.git.ps@pks.im>
On 2024.03.05 13:11, Patrick Steinhardt wrote:
Show 31 quoted lines
> 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@pks.im>
> ---
>  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]
Show 27 quoted lines
> 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?

Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 15 of 20 in “reftable: memory optimizations for reflog iteration”
  1. 0/7 reftable: memory optimizations for reflog iterationPatrick Steinhardt, Mar 5, 2024
  2. 1/7 refs/reftable: reload correct stack when creating reflog iterPatrick Steinhardt, Mar 5, 2024
  3. Karthik NayakMar 6, 2024
  4. Junio C HamanoMar 6, 2024
  5. Patrick SteinhardtMar 7, 2024
  6. Josh SteadmonMar 11, 2024
  7. Patrick SteinhardtMar 11, 2024
  8. 2/7 reftable/record: convert old and new object IDs to arraysPatrick Steinhardt, Mar 5, 2024
  9. 3/7 reftable/record: avoid copying author infoPatrick Steinhardt, Mar 5, 2024
  10. James LiuMar 13, 2024
  11. Patrick SteinhardtMar 21, 2024
  12. 4/7 reftable/record: reuse refnames when decoding log recordsPatrick Steinhardt, Mar 5, 2024
  13. 5/7 reftable/record: reuse message when decoding log recordsPatrick Steinhardt, Mar 5, 2024
  14. 6/7 reftable/record: use scratch buffer when decoding recordsPatrick Steinhardt, Mar 5, 2024
  15. Josh SteadmonMar 11, 2024
  16. Patrick SteinhardtMar 11, 2024
  17. Josh SteadmonMar 11, 2024
  18. 7/7 refs/reftable: track last log record name via strbufPatrick Steinhardt, Mar 5, 2024
  19. Josh SteadmonMar 11, 2024
  20. Patrick SteinhardtMar 11, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.