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

[PATCH v3 8/8] reftable/merged: transfer ownership of records when iterating

From
Patrick Steinhardt <ps@pks.im>
Date
Jan 3, 2024, 06:22 UTC
Message-ID
<eea0e161adca68e986cd7af0a195e60b8d4cbc9f.1704262787.git.ps@pks.im>
In-Reply-To
<cover.1704262787.git.ps@pks.im>

When iterating over records with the merged iterator we put the records into a priority queue before yielding them to the caller. This means that we need to allocate the contents of these records before we can pass them over to the caller.

The handover to the caller is quite inefficient though because we first deallocate the record passed in by the caller and then copy over the new record, which requires us to reallocate memory.

Refactor the code to instead transfer ownership of the new record to the caller. So instead of reallocating all contents, we now release the old record and then copy contents of the new record into place.

The following benchmark of `git show-ref --quiet` in a repository with around 350k refs shows a clear improvement. Before:

    HEAP SUMMARY:
        in use at exit: 21,163 bytes in 193 blocks
      total heap usage: 708,058 allocs, 707,865 frees, 36,783,255 bytes allocated
After:
    HEAP SUMMARY:
        in use at exit: 21,163 bytes in 193 blocks
      total heap usage: 357,007 allocs, 356,814 frees, 24,193,602 bytes allocated

This shows that we now have roundabout a single allocation per record that we're yielding from the iterator. Ideally, we'd also get rid of this allocation so that the number of allocations doesn't scale with the number of refs anymore. This would require some larger surgery though because the memory is owned by the priority queue before transferring it over to the caller.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/merged.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/reftable/merged.c b/reftable/merged.c
index a28bb99aaf..a52914d667 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -124,10 +124,12 @@ static int merged_iter_next_entry(struct merged_iter *mi,
 		reftable_record_release(&top.rec);
 	}
 
-	reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id));
+	reftable_record_release(rec);
+	*rec = entry.rec;
 
 done:
-	reftable_record_release(&entry.rec);
+	if (err)
+		reftable_record_release(&entry.rec);
 	return err;
 }
 
-- 
2.43.GIT
Previous: Patrick SteinhardtNext: Karthik Nayak
Message 35 of 36 in “reftable: fixes and optimizations (pt.2)”
  1. 0/7 reftable: fixes and optimizations (pt.2)Patrick Steinhardt, Dec 20, 2023
  2. 1/7 reftable/stack: do not overwrite errors when compactingPatrick Steinhardt, Dec 20, 2023
  3. 2/7 reftable/writer: fix index corruption when writing multiple indicesPatrick Steinhardt, Dec 20, 2023
  4. 3/7 reftable/record: constify some parts of the interfacePatrick Steinhardt, Dec 20, 2023
  5. 4/7 reftable/record: store "val1" hashes as static arraysPatrick Steinhardt, Dec 20, 2023
  6. Patrick SteinhardtDec 20, 2023
  7. 5/7 reftable/record: store "val2" hashes as static arraysPatrick Steinhardt, Dec 20, 2023
  8. 6/7 reftable/merged: really reuse buffers to compute record keysPatrick Steinhardt, Dec 20, 2023
  9. 7/7 reftable/merged: transfer ownership of records when iteratingPatrick Steinhardt, Dec 20, 2023
  10. Junio C HamanoDec 20, 2023
  11. 0/8 reftable: fixes and optimizations (pt.2)Patrick Steinhardt, Dec 28, 2023
  12. 1/8 reftable/stack: do not overwrite errors when compactingPatrick Steinhardt, Dec 28, 2023
  13. 2/8 reftable/stack: do not auto-compact twice in `reftable_stack_add()`Patrick Steinhardt, Dec 28, 2023
  14. 3/8 reftable/writer: fix index corruption when writing multiple indicesPatrick Steinhardt, Dec 28, 2023
  15. 4/8 reftable/record: constify some parts of the interfacePatrick Steinhardt, Dec 28, 2023
  16. 5/8 reftable/record: store "val1" hashes as static arraysPatrick Steinhardt, Dec 28, 2023
  17. Junio C HamanoDec 28, 2023
  18. 6/8 reftable/record: store "val2" hashes as static arraysPatrick Steinhardt, Dec 28, 2023
  19. 7/8 reftable/merged: really reuse buffers to compute record keysPatrick Steinhardt, Dec 28, 2023
  20. Junio C HamanoDec 28, 2023
  21. 8/8 reftable/merged: transfer ownership of records when iteratingPatrick Steinhardt, Dec 28, 2023
  22. Junio C HamanoDec 28, 2023
  23. 0/8 reftable: fixes and optimizations (pt.2)Patrick Steinhardt, Jan 3, 2024
  24. 1/8 reftable/stack: do not overwrite errors when compactingPatrick Steinhardt, Jan 3, 2024
  25. Han-Wen NienhuysFeb 14, 2024
  26. Patrick SteinhardtFeb 15, 2024
  27. 2/8 reftable/stack: do not auto-compact twice in `reftable_stack_add()`Patrick Steinhardt, Jan 3, 2024
  28. 3/8 reftable/writer: fix index corruption when writing multiple indicesPatrick Steinhardt, Jan 3, 2024
  29. 4/8 reftable/record: constify some parts of the interfacePatrick Steinhardt, Jan 3, 2024
  30. 5/8 reftable/record: store "val1" hashes as static arraysPatrick Steinhardt, Jan 3, 2024
  31. Karthik NayakFeb 5, 2024
  32. Patrick SteinhardtFeb 6, 2024
  33. 6/8 reftable/record: store "val2" hashes as static arraysPatrick Steinhardt, Jan 3, 2024
  34. 7/8 reftable/merged: really reuse buffers to compute record keysPatrick Steinhardt, Jan 3, 2024
  35. 8/8 reftable/merged: transfer ownership of records when iteratingPatrick Steinhardt, Jan 3, 2024
  36. Karthik NayakFeb 5, 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.