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

[PATCH v2 7/7] reftable/reader: add comments to `table_iter_next()`

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

While working on the optimizations in the preceding patches I stumbled upon `table_iter_next()` multiple times. It is quite easy to miss the fact that we don't call `table_iter_next_in_block()` twice, but that the second call is in fact `table_iter_next_block()`.

Add comments to explain what exactly is going on here to make things more obvious. While at it, touch up the code to conform to our code style better.

Note that one of the refactorings merges two conditional blocks into one. Before, we had the following code:

```
err = table_iter_next_block(&next, ti
if (err != 0) {
	ti->is_finished = 1;
}
table_iter_block_done(ti);
if (err != 0) {
	return err;
}
```

As `table_iter_block_done()` does not care about `is_finished`, the conditional blocks can be merged into one block:

```
err = table_iter_next_block(&next, ti
table_iter_block_done(ti);
if (err != 0) {
	ti->is_finished = 1;
	return err;
}
```

This is both easier to reason about and more performant because we have one branch less.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/reader.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/reftable/reader.c b/reftable/reader.c
index 64dc366fb1..add7d57f0b 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -357,24 +357,32 @@ static int table_iter_next(struct table_iter *ti, struct reftable_record *rec)
 
 	while (1) {
 		struct table_iter next = TABLE_ITER_INIT;
-		int err = 0;
-		if (ti->is_finished) {
+		int err;
+
+		if (ti->is_finished)
 			return 1;
-		}
 
+		/*
+		 * Check whether the current block still has more records. If
+		 * so, return it. If the iterator returns positive then the
+		 * current block has been exhausted.
+		 */
 		err = table_iter_next_in_block(ti, rec);
-		if (err <= 0) {
+		if (err <= 0)
 			return err;
-		}
 
+		/*
+		 * Otherwise, we need to continue to the next block in the
+		 * table and retry. If there are no more blocks then the
+		 * iterator is drained.
+		 */
 		err = table_iter_next_block(&next, ti);
-		if (err != 0) {
-			ti->is_finished = 1;
-		}
 		table_iter_block_done(ti);
-		if (err != 0) {
+		if (err) {
+			ti->is_finished = 1;
 			return err;
 		}
+
 		table_iter_copy_from(ti, &next);
 		block_iter_close(&next.bi);
 	}
-- 
2.43.GIT
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 21 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.