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

[PATCH 6/9] reftable/writer: refactorings for `writer_flush_nonempty_block()`

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 2, 2024, 17:30 UTC
Message-ID
<1f903afdda229ae3e6b73c5612d77f4647079690.1712078736.git.ps@pks.im>
In-Reply-To
<cover.1712078736.git.ps@pks.im>

Large parts of the reftable library do not conform to Git's typical code style. Refactor `writer_flush_nonempty_block()` such that it conforms better to it and add some documentation that explains some of its more intricate behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/writer.c | 72 +++++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 28 deletions(-)
diff --git a/reftable/writer.c b/reftable/writer.c
index 0ad5eb8887..d347ec4cc6 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -659,58 +659,74 @@ static void writer_clear_index(struct reftable_writer *w)
 	w->index_cap = 0;
 }
 
-static const int debug = 0;
-
 static int writer_flush_nonempty_block(struct reftable_writer *w)
 {
+	struct reftable_index_record index_record = {
+		.last_key = STRBUF_INIT,
+	};
 	uint8_t typ = block_writer_type(w->block_writer);
-	struct reftable_block_stats *bstats =
-		writer_reftable_block_stats(w, typ);
-	uint64_t block_typ_off = (bstats->blocks == 0) ? w->next : 0;
-	int raw_bytes = block_writer_finish(w->block_writer);
-	int padding = 0;
-	int err = 0;
-	struct reftable_index_record ir = { .last_key = STRBUF_INIT };
+	struct reftable_block_stats *bstats;
+	int raw_bytes, padding = 0, err;
+	uint64_t block_typ_off;
+
+	/*
+	 * Finish the current block. This will cause the block writer to emit
+	 * restart points and potentially compress records in case we are
+	 * writing a log block.
+	 *
+	 * Note that this is still happening in memory.
+	 */
+	raw_bytes = block_writer_finish(w->block_writer);
 	if (raw_bytes < 0)
 		return raw_bytes;
 
-	if (!w->opts.unpadded && typ != BLOCK_TYPE_LOG) {
+	/*
+	 * By default, all records except for log records are padded to the
+	 * block size.
+	 */
+	if (!w->opts.unpadded && typ != BLOCK_TYPE_LOG)
 		padding = w->opts.block_size - raw_bytes;
-	}
 
-	if (block_typ_off > 0) {
+	bstats = writer_reftable_block_stats(w, typ);
+	block_typ_off = (bstats->blocks == 0) ? w->next : 0;
+	if (block_typ_off > 0)
 		bstats->offset = block_typ_off;
-	}
-
 	bstats->entries += w->block_writer->entries;
 	bstats->restarts += w->block_writer->restart_len;
 	bstats->blocks++;
 	w->stats.blocks++;
 
-	if (debug) {
-		fprintf(stderr, "block %c off %" PRIu64 " sz %d (%d)\n", typ,
-			w->next, raw_bytes,
-			get_be24(w->block + w->block_writer->header_off + 1));
-	}
-
-	if (w->next == 0) {
+	/*
+	 * If this is the first block we're writing to the table then we need
+	 * to also write the reftable header.
+	 */
+	if (!w->next)
 		writer_write_header(w, w->block);
-	}
 
 	err = padded_write(w, w->block, raw_bytes, padding);
 	if (err < 0)
 		return err;
 
+	/*
+	 * Add an index record for every block that we're writing. If we end up
+	 * having more than a threshold of index records we will end up writing
+	 * an index section in `writer_finish_section()`. Each index record
+	 * contains the last record key of the block it is indexing as well as
+	 * the offset of that block.
+	 *
+	 * Note that this also applies when flushing index blocks, in which
+	 * case we will end up with a multi-level index.
+	 */
 	REFTABLE_ALLOC_GROW(w->index, w->index_len + 1, w->index_cap);
-
-	ir.offset = w->next;
-	strbuf_reset(&ir.last_key);
-	strbuf_addbuf(&ir.last_key, &w->block_writer->last_key);
-	w->index[w->index_len] = ir;
-
+	index_record.offset = w->next;
+	strbuf_reset(&index_record.last_key);
+	strbuf_addbuf(&index_record.last_key, &w->block_writer->last_key);
+	w->index[w->index_len] = index_record;
 	w->index_len++;
+
 	w->next += padding + raw_bytes;
 	w->block_writer = NULL;
+
 	return 0;
 }
 
-- 
2.44.GIT
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 10 of 49 in “reftable: optimize write performance”
  1. 0/9 reftable: optimize write performancePatrick Steinhardt, Apr 2, 2024
  2. 1/9 refs/reftable: fix D/F conflict error message on ref copyPatrick Steinhardt, Apr 2, 2024
  3. Junio C HamanoApr 3, 2024
  4. 2/9 refs/reftable: perform explicit D/F check when writing symrefsPatrick Steinhardt, Apr 2, 2024
  5. 3/9 refs/reftable: skip duplicate name checksPatrick Steinhardt, Apr 2, 2024
  6. 4/9 refs/reftable: don't recompute committer identPatrick Steinhardt, Apr 2, 2024
  7. Junio C HamanoApr 3, 2024
  8. Patrick SteinhardtApr 4, 2024
  9. 5/9 reftable/writer: refactorings for `writer_add_record()`Patrick Steinhardt, Apr 2, 2024
  10. 6/9 reftable/writer: refactorings for `writer_flush_nonempty_block()`Patrick Steinhardt, Apr 2, 2024
  11. 7/9 reftable/block: reuse zstream when writing log blocksPatrick Steinhardt, Apr 2, 2024
  12. Junio C HamanoApr 3, 2024
  13. Patrick SteinhardtApr 4, 2024
  14. 8/9 reftable/block: reuse compressed arrayPatrick Steinhardt, Apr 2, 2024
  15. 9/9 reftable/writer: reset `last_key` instead of releasing itPatrick Steinhardt, Apr 2, 2024
  16. 00/11 reftable: optimize write performancePatrick Steinhardt, Apr 4, 2024
  17. 01/11 refs/reftable: fix D/F conflict error message on ref copyPatrick Steinhardt, Apr 4, 2024
  18. 02/11 refs/reftable: perform explicit D/F check when writing symrefsPatrick Steinhardt, Apr 4, 2024
  19. 03/11 refs/reftable: skip duplicate name checksPatrick Steinhardt, Apr 4, 2024
  20. 04/11 reftable: remove name checksPatrick Steinhardt, Apr 4, 2024
  21. 05/11 refs/reftable: don't recompute committer identPatrick Steinhardt, Apr 4, 2024
  22. 06/11 reftable/writer: refactorings for `writer_add_record()`Patrick Steinhardt, Apr 4, 2024
  23. Han-Wen NienhuysApr 4, 2024
  24. Patrick SteinhardtApr 4, 2024
  25. 07/11 reftable/writer: refactorings for `writer_flush_nonempty_block()`Patrick Steinhardt, Apr 4, 2024
  26. 08/11 reftable/writer: unify releasing memoryPatrick Steinhardt, Apr 4, 2024
  27. Han-Wen NienhuysApr 4, 2024
  28. Patrick SteinhardtApr 4, 2024
  29. Han-Wen NienhuysApr 4, 2024
  30. Patrick SteinhardtApr 4, 2024
  31. 09/11 reftable/writer: reset `last_key` instead of releasing itPatrick Steinhardt, Apr 4, 2024
  32. 10/11 reftable/block: reuse zstream when writing log blocksPatrick Steinhardt, Apr 4, 2024
  33. 11/11 reftable/block: reuse compressed arrayPatrick Steinhardt, Apr 4, 2024
  34. Han-Wen NienhuysApr 4, 2024
  35. Patrick SteinhardtApr 4, 2024
  36. 00/11 reftable: optimize write performancePatrick Steinhardt, Apr 8, 2024
  37. 01/11 refs/reftable: fix D/F conflict error message on ref copyPatrick Steinhardt, Apr 8, 2024
  38. 02/11 refs/reftable: perform explicit D/F check when writing symrefsPatrick Steinhardt, Apr 8, 2024
  39. 03/11 refs/reftable: skip duplicate name checksPatrick Steinhardt, Apr 8, 2024
  40. 04/11 reftable: remove name checksPatrick Steinhardt, Apr 8, 2024
  41. 05/11 refs/reftable: don't recompute committer identPatrick Steinhardt, Apr 8, 2024
  42. 06/11 reftable/writer: refactorings for `writer_add_record()`Patrick Steinhardt, Apr 8, 2024
  43. 07/11 reftable/writer: refactorings for `writer_flush_nonempty_block()`Patrick Steinhardt, Apr 8, 2024
  44. 08/11 reftable/writer: unify releasing memoryPatrick Steinhardt, Apr 8, 2024
  45. 09/11 reftable/writer: reset `last_key` instead of releasing itPatrick Steinhardt, Apr 8, 2024
  46. 10/11 reftable/block: reuse zstream when writing log blocksPatrick Steinhardt, Apr 8, 2024
  47. 11/11 reftable/block: reuse compressed arrayPatrick Steinhardt, Apr 8, 2024
  48. Junio C HamanoApr 9, 2024
  49. Patrick SteinhardtApr 9, 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.