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

[PATCH 02/13] reftable/reader: avoid copying index iterator

From
Patrick Steinhardt <ps@pks.im>
Date
May 8, 2024, 11:03 UTC
Message-ID
<bdbebdbd363fc264c6327e7bf0af29ee5357e83d.1715166175.git.ps@pks.im>
In-Reply-To
<cover.1715166175.git.ps@pks.im>

When doing an indexed seek we need to walk down the multi-level index until we finally hit a record of the desired indexed type. This loop performs a copy of the index iterator on every iteration, which is both hard to understand and completely unnecessary.

Refactor the code so that we use a single iterator to walk down the indices, only.

Note that while this should improve performance, the improvement is negligible in all but the most unreasonable repositories. This is because the effect is only really noticeable when we have to walk down many levels of indices, which is not something that a repository would typically have. So the motivation for this change is really only about readability.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/reader.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/reftable/reader.c b/reftable/reader.c
index 481dff10d4..6bfadcad71 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -510,13 +510,11 @@ static int reader_seek_indexed(struct reftable_reader *r,
 		.type = BLOCK_TYPE_INDEX,
 		.u.idx = { .last_key = STRBUF_INIT },
 	};
-	struct table_iter index_iter = TABLE_ITER_INIT;
-	struct table_iter empty = TABLE_ITER_INIT;
-	struct table_iter next = TABLE_ITER_INIT;
+	struct table_iter ti = TABLE_ITER_INIT, *malloced;
 	int err = 0;
 
 	reftable_record_key(rec, &want_index.u.idx.last_key);
-	err = reader_start(r, &index_iter, reftable_record_type(rec), 1);
+	err = reader_start(r, &ti, reftable_record_type(rec), 1);
 	if (err < 0)
 		goto done;
 
@@ -526,7 +524,7 @@ static int reader_seek_indexed(struct reftable_reader *r,
 	 * highest layer that identifies the relevant index block as well as
 	 * the record inside that block that corresponds to our wanted key.
 	 */
-	err = reader_seek_linear(&index_iter, &want_index);
+	err = reader_seek_linear(&ti, &want_index);
 	if (err < 0)
 		goto done;
 
@@ -552,44 +550,36 @@ static int reader_seek_indexed(struct reftable_reader *r,
 		 * all levels of the index only to find out that the key does
 		 * not exist.
 		 */
-		err = table_iter_next(&index_iter, &index_result);
+		err = table_iter_next(&ti, &index_result);
 		if (err != 0)
 			goto done;
 
-		err = reader_table_iter_at(r, &next, index_result.u.idx.offset,
-					   0);
+		err = reader_table_iter_at(r, &ti, index_result.u.idx.offset, 0);
 		if (err != 0)
 			goto done;
 
-		err = block_iter_seek_key(&next.bi, &next.br, &want_index.u.idx.last_key);
+		err = block_iter_seek_key(&ti.bi, &ti.br, &want_index.u.idx.last_key);
 		if (err < 0)
 			goto done;
 
-		if (next.typ == reftable_record_type(rec)) {
+		if (ti.typ == reftable_record_type(rec)) {
 			err = 0;
 			break;
 		}
 
-		if (next.typ != BLOCK_TYPE_INDEX) {
+		if (ti.typ != BLOCK_TYPE_INDEX) {
 			err = REFTABLE_FORMAT_ERROR;
-			break;
+			goto done;
 		}
-
-		table_iter_close(&index_iter);
-		index_iter = next;
-		next = empty;
 	}
 
-	if (err == 0) {
-		struct table_iter *malloced = reftable_calloc(1, sizeof(*malloced));
-		*malloced = next;
-		next = empty;
-		iterator_from_table_iter(it, malloced);
-	}
+	REFTABLE_ALLOC_ARRAY(malloced, 1);
+	*malloced = ti;
+	iterator_from_table_iter(it, malloced);
 
 done:
-	table_iter_close(&next);
-	table_iter_close(&index_iter);
+	if (err)
+		table_iter_close(&ti);
 	reftable_record_release(&want_index);
 	reftable_record_release(&index_result);
 	return err;
-- 
2.45.0
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 3 of 50 in “reftable: prepare for re-seekable iterators”
  1. 00/13 reftable: prepare for re-seekable iteratorsPatrick Steinhardt, May 8, 2024
  2. 01/13 reftable/block: use `size_t` to track restart point indexPatrick Steinhardt, May 8, 2024
  3. 02/13 reftable/reader: avoid copying index iteratorPatrick Steinhardt, May 8, 2024
  4. 03/13 reftable/reader: unify indexed and linear seekingPatrick Steinhardt, May 8, 2024
  5. 04/13 reftable/reader: separate concerns of table iter and reftable readerPatrick Steinhardt, May 8, 2024
  6. 05/13 reftable/reader: inline `reader_seek_internal()`Patrick Steinhardt, May 8, 2024
  7. 06/13 reftable/reader: set up the reader when initializing table iteratorPatrick Steinhardt, May 8, 2024
  8. 07/13 reftable/merged: split up initialization and seeking of recordsPatrick Steinhardt, May 8, 2024
  9. Justin ToblerMay 10, 2024
  10. Patrick SteinhardtMay 13, 2024
  11. 08/13 reftable/merged: simplify indices for subiteratorsPatrick Steinhardt, May 8, 2024
  12. Justin ToblerMay 10, 2024
  13. Patrick SteinhardtMay 13, 2024
  14. 09/13 reftable/generic: move seeking of records into the iteratorPatrick Steinhardt, May 8, 2024
  15. Justin ToblerMay 10, 2024
  16. Patrick SteinhardtMay 13, 2024
  17. 10/13 reftable/generic: adapt interface to allow reuse of iteratorsPatrick Steinhardt, May 8, 2024
  18. 11/13 reftable/reader: adapt interface to allow reuse of iteratorsPatrick Steinhardt, May 8, 2024
  19. Justin ToblerMay 10, 2024
  20. Patrick SteinhardtMay 13, 2024
  21. 12/13 reftable/stack: provide convenience functions to create iteratorsPatrick Steinhardt, May 8, 2024
  22. 13/13 reftable/merged: adapt interface to allow reuse of iteratorsPatrick Steinhardt, May 8, 2024
  23. Junio C HamanoMay 8, 2024
  24. Junio C HamanoMay 9, 2024
  25. Patrick SteinhardtMay 10, 2024
  26. Junio C HamanoMay 10, 2024
  27. Patrick SteinhardtMay 10, 2024
  28. Junio C HamanoMay 10, 2024
  29. 00/13 reftable: prepare for re-seekable iteratorsPatrick Steinhardt, May 13, 2024
  30. 01/13 reftable/block: use `size_t` to track restart point indexPatrick Steinhardt, May 13, 2024
  31. Karthik NayakMay 21, 2024
  32. Patrick SteinhardtMay 22, 2024
  33. 02/13 reftable/reader: avoid copying index iteratorPatrick Steinhardt, May 13, 2024
  34. 03/13 reftable/reader: unify indexed and linear seekingPatrick Steinhardt, May 13, 2024
  35. Karthik NayakMay 21, 2024
  36. Patrick SteinhardtMay 22, 2024
  37. Karthik NayakMay 22, 2024
  38. 04/13 reftable/reader: separate concerns of table iter and reftable readerPatrick Steinhardt, May 13, 2024
  39. 05/13 reftable/reader: inline `reader_seek_internal()`Patrick Steinhardt, May 13, 2024
  40. 06/13 reftable/reader: set up the reader when initializing table iteratorPatrick Steinhardt, May 13, 2024
  41. 07/13 reftable/merged: split up initialization and seeking of recordsPatrick Steinhardt, May 13, 2024
  42. 08/13 reftable/merged: simplify indices for subiteratorsPatrick Steinhardt, May 13, 2024
  43. 09/13 reftable/generic: move seeking of records into the iteratorPatrick Steinhardt, May 13, 2024
  44. 10/13 reftable/generic: adapt interface to allow reuse of iteratorsPatrick Steinhardt, May 13, 2024
  45. 11/13 reftable/reader: adapt interface to allow reuse of iteratorsPatrick Steinhardt, May 13, 2024
  46. 12/13 reftable/stack: provide convenience functions to create iteratorsPatrick Steinhardt, May 13, 2024
  47. 13/13 reftable/merged: adapt interface to allow reuse of iteratorsPatrick Steinhardt, May 13, 2024
  48. Justin ToblerMay 15, 2024
  49. Karthik NayakMay 21, 2024
  50. Patrick SteinhardtMay 22, 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.