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

[PATCH v2 2/7] reftable/merged: allocation-less dropping of shadowed records

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 12, 2024, 08:32 UTC
Message-ID
<2364a0fa3369063355bed966267a19fb9af3a6b6.1707726654.git.ps@pks.im>
In-Reply-To
<cover.1707726654.git.ps@pks.im>

The purpose of the merged reftable iterator is to iterate through all entries of a set of tables in the correct order. This is implemented by using a sub-iterator for each table, where the next entry of each of these iterators gets put into a priority queue. For each iteration, we do roughly the following steps:

  1. Retrieve the top record of the priority queue. This is the entry we
     want to return to the caller.
  2. Retrieve the next record of the sub-iterator that this record came
     from. If any, add it to the priority queue at the correct position.
     The position is determined by comparing the record keys, which e.g.
     corresponds to the refname for ref records.
  3. Keep removing the top record of the priority queue until we hit the
     first entry whose key is larger than the returned record's key.
     This is required to drop "shadowed" records.

The last step will lead to at least one comparison to the next entry, but may lead to many comparisons in case the reftable stack consists of many tables with shadowed records. It is thus part of the hot code path when iterating through records.

The code to compare the entries with each other is quite inefficient though. Instead of comparing record keys with each other directly, we first format them into `struct strbuf`s and only then compare them with each other. While we already optimized this code path to reuse buffers in 829231dc20 (reftable/merged: reuse buffer to compute record keys, 2023-12-11), the cost to format the keys into the buffers still adds up quite significantly.

Refactor the code to use `reftable_record_cmp()` instead, which has been introduced in the preceding commit. This function compares records with each other directly without requiring any memory allocations or copying and is thus way more efficient.

The following benchmark uses git-show-ref(1) to print a single ref matching a pattern out of 1 million refs. This is the most direct way to exercise ref iteration speed as we remove all overhead of having to show the refs, too.

    Benchmark 1: show-ref: single matching ref (revision = HEAD~)
      Time (mean ± σ):     180.7 ms ±   4.7 ms    [User: 177.1 ms, System: 3.4 ms]
      Range (min … max):   174.9 ms … 211.7 ms    1000 runs
    Benchmark 2: show-ref: single matching ref (revision = HEAD)
      Time (mean ± σ):     162.1 ms ±   4.4 ms    [User: 158.5 ms, System: 3.4 ms]
      Range (min … max):   155.4 ms … 189.3 ms    1000 runs
    Summary
      show-ref: single matching ref (revision = HEAD) ran
        1.11 ± 0.04 times faster than show-ref: single matching ref (revision = HEAD~)
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/merged.c | 11 ++---------
 reftable/merged.h |  2 --
 2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/reftable/merged.c b/reftable/merged.c
index c258ce953e..fb9978d798 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -51,8 +51,6 @@ static void merged_iter_close(void *p)
 		reftable_iterator_destroy(&mi->stack[i]);
 	}
 	reftable_free(mi->stack);
-	strbuf_release(&mi->key);
-	strbuf_release(&mi->entry_key);
 }
 
 static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
@@ -105,14 +103,11 @@ static int merged_iter_next_entry(struct merged_iter *mi,
 	  such a deployment, the loop below must be changed to collect all
 	  entries for the same key, and return new the newest one.
 	*/
-	reftable_record_key(&entry.rec, &mi->entry_key);
 	while (!merged_iter_pqueue_is_empty(mi->pq)) {
 		struct pq_entry top = merged_iter_pqueue_top(mi->pq);
-		int cmp = 0;
+		int cmp;
 
-		reftable_record_key(&top.rec, &mi->key);
-
-		cmp = strbuf_cmp(&mi->key, &mi->entry_key);
+		cmp = reftable_record_cmp(&top.rec, &entry.rec);
 		if (cmp > 0)
 			break;
 
@@ -246,8 +241,6 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
 		.typ = reftable_record_type(rec),
 		.hash_id = mt->hash_id,
 		.suppress_deletions = mt->suppress_deletions,
-		.key = STRBUF_INIT,
-		.entry_key = STRBUF_INIT,
 	};
 	int n = 0;
 	int err = 0;
diff --git a/reftable/merged.h b/reftable/merged.h
index d5b39dfe7f..7d9f95d27e 100644
--- a/reftable/merged.h
+++ b/reftable/merged.h
@@ -31,8 +31,6 @@ struct merged_iter {
 	uint8_t typ;
 	int suppress_deletions;
 	struct merged_iter_pqueue pq;
-	struct strbuf key;
-	struct strbuf entry_key;
 };
 
 void merged_table_release(struct reftable_merged_table *mt);
-- 
2.43.GIT
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 16 of 23 in “reftable: improve ref iteration performance”
  1. 0/7 reftable: improve ref iteration performancePatrick Steinhardt, Feb 1, 2024
  2. 1/7 reftable/record: introduce function to compare records by keyPatrick Steinhardt, Feb 1, 2024
  3. Eric SunshineFeb 1, 2024
  4. 2/7 reftable/merged: allocation-less dropping of shadowed recordsPatrick Steinhardt, Feb 1, 2024
  5. 3/7 reftable/merged: skip comparison for records of the same subiterPatrick Steinhardt, Feb 1, 2024
  6. Eric SunshineFeb 1, 2024
  7. Patrick SteinhardtFeb 2, 2024
  8. 4/7 reftable/pq: allocation-less comparison of entry keysPatrick Steinhardt, Feb 1, 2024
  9. 5/7 reftable/block: swap buffers instead of copyingPatrick Steinhardt, Feb 1, 2024
  10. 6/7 reftable/record: don't try to reallocate ref record namePatrick Steinhardt, Feb 1, 2024
  11. 7/7 reftable/reader: add comments to `table_iter_next()`Patrick Steinhardt, Feb 1, 2024
  12. John CaiFeb 9, 2024
  13. Patrick SteinhardtFeb 12, 2024
  14. 0/7 reftable: improve ref iteration performancePatrick Steinhardt, Feb 12, 2024
  15. 1/7 reftable/record: introduce function to compare records by keyPatrick Steinhardt, Feb 12, 2024
  16. 2/7 reftable/merged: allocation-less dropping of shadowed recordsPatrick Steinhardt, Feb 12, 2024
  17. 3/7 reftable/merged: skip comparison for records of the same subiterPatrick Steinhardt, Feb 12, 2024
  18. 4/7 reftable/pq: allocation-less comparison of entry keysPatrick Steinhardt, Feb 12, 2024
  19. 5/7 reftable/block: swap buffers instead of copyingPatrick Steinhardt, Feb 12, 2024
  20. 6/7 reftable/record: don't try to reallocate ref record namePatrick Steinhardt, Feb 12, 2024
  21. 7/7 reftable/reader: add comments to `table_iter_next()`Patrick Steinhardt, Feb 12, 2024
  22. Junio C HamanoFeb 12, 2024
  23. Patrick SteinhardtFeb 13, 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.