From b8ca235ca5fad0421766db7fe2c0f52c6707bf8e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 22 Aug 2024 08:34:44 +0200 Subject: reftable/merged: stop using generic tables in the merged table The merged table provides access to a reftable stack by merging the contents of those tables into a virtual table. These subtables are being tracked via `struct reftable_table`, which is a generic interface for accessing either a single reftable or a merged reftable. So in theory, it would be possible for the merged table to merge together other merged tables. This is somewhat nonsensical though: we only ever set up a merged table over normal reftables, and there is no reason to do otherwise. This generic interface thus makes the code way harder to follow and reason about than really necessary. The abstraction layer may also have an impact on performance, even though the extra set of vtable function calls probably doesn't really matter. Refactor the merged tables to use a `struct reftable_reader` for each of the subtables instead, which gives us direct access to the underlying tables. Adjust names accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/stack_test.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'reftable/stack_test.c') diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 8c36590ff0..dbca9eaf4a 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -347,9 +347,9 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void) * all tables in the stack. */ if (i != n) - EXPECT(st->merged->stack_len == i + 1); + EXPECT(st->merged->readers_len == i + 1); else - EXPECT(st->merged->stack_len == 1); + EXPECT(st->merged->readers_len == 1); } reftable_stack_destroy(st); @@ -375,7 +375,7 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(void) err = reftable_stack_add(st, write_test_ref, &ref); EXPECT_ERR(err); - EXPECT(st->merged->stack_len == 1); + EXPECT(st->merged->readers_len == 1); EXPECT(st->stats.attempts == 0); EXPECT(st->stats.failures == 0); @@ -390,7 +390,7 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(void) ref.update_index = 2; err = reftable_stack_add(st, write_test_ref, &ref); EXPECT_ERR(err); - EXPECT(st->merged->stack_len == 2); + EXPECT(st->merged->readers_len == 2); EXPECT(st->stats.attempts == 1); EXPECT(st->stats.failures == 1); @@ -881,7 +881,7 @@ static void test_reftable_stack_auto_compaction(void) err = reftable_stack_auto_compact(st); EXPECT_ERR(err); - EXPECT(i < 3 || st->merged->stack_len < 2 * fastlog2(i)); + EXPECT(i < 3 || st->merged->readers_len < 2 * fastlog2(i)); } EXPECT(reftable_stack_compaction_stats(st)->entries_written < @@ -905,7 +905,7 @@ static void test_reftable_stack_auto_compaction_with_locked_tables(void) EXPECT_ERR(err); write_n_ref_tables(st, 5); - EXPECT(st->merged->stack_len == 5); + EXPECT(st->merged->readers_len == 5); /* * Given that all tables we have written should be roughly the same @@ -925,7 +925,7 @@ static void test_reftable_stack_auto_compaction_with_locked_tables(void) err = reftable_stack_auto_compact(st); EXPECT_ERR(err); EXPECT(st->stats.failures == 0); - EXPECT(st->merged->stack_len == 4); + EXPECT(st->merged->readers_len == 4); reftable_stack_destroy(st); strbuf_release(&buf); @@ -970,9 +970,9 @@ static void test_reftable_stack_add_performs_auto_compaction(void) * all tables in the stack. */ if (i != n) - EXPECT(st->merged->stack_len == i + 1); + EXPECT(st->merged->readers_len == i + 1); else - EXPECT(st->merged->stack_len == 1); + EXPECT(st->merged->readers_len == 1); } reftable_stack_destroy(st); @@ -994,7 +994,7 @@ static void test_reftable_stack_compaction_with_locked_tables(void) EXPECT_ERR(err); write_n_ref_tables(st, 3); - EXPECT(st->merged->stack_len == 3); + EXPECT(st->merged->readers_len == 3); /* Lock one of the tables that we're about to compact. */ strbuf_reset(&buf); @@ -1008,7 +1008,7 @@ static void test_reftable_stack_compaction_with_locked_tables(void) err = reftable_stack_compact_all(st, NULL); EXPECT(err == REFTABLE_LOCK_ERROR); EXPECT(st->stats.failures == 1); - EXPECT(st->merged->stack_len == 3); + EXPECT(st->merged->readers_len == 3); reftable_stack_destroy(st); strbuf_release(&buf); -- cgit v1.3 From ca74ef6ffb7388d862379d5016282340aff1f68b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 22 Aug 2024 08:35:12 +0200 Subject: t/helper: inline `reftable_stack_print_directory()` Move `reftable_stack_print_directory()` into the "dump-reftable" helper. This follows the same reasoning as the preceding commit. Note that this requires us to remove the tests for this functionality in `reftable/stack_test.c`. The test does not really add much anyway, because all it verifies is that we do not crash or run into an error, and it specifically doesn't check the outputted data. Also, as the code is now part of the test helper, it doesn't make much sense to have a unit test for it in the first place. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/reftable-stack.h | 3 --- reftable/stack.c | 20 -------------------- reftable/stack_test.c | 7 ------- t/helper/test-reftable.c | 23 ++++++++++++++++++++++- 4 files changed, 22 insertions(+), 31 deletions(-) (limited to 'reftable/stack_test.c') diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h index 09e97c9991..f4f8cabc7f 100644 --- a/reftable/reftable-stack.h +++ b/reftable/reftable-stack.h @@ -140,7 +140,4 @@ struct reftable_compaction_stats { struct reftable_compaction_stats * reftable_stack_compaction_stats(struct reftable_stack *st); -/* print the entire stack represented by the directory */ -int reftable_stack_print_directory(const char *stackdir, uint32_t hash_id); - #endif diff --git a/reftable/stack.c b/reftable/stack.c index d08ec00959..bedd503e7e 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1603,23 +1603,3 @@ done: reftable_addition_destroy(add); return err; } - -int reftable_stack_print_directory(const char *stackdir, uint32_t hash_id) -{ - struct reftable_stack *stack = NULL; - struct reftable_write_options opts = { .hash_id = hash_id }; - struct reftable_merged_table *merged = NULL; - struct reftable_table table = { NULL }; - - int err = reftable_new_stack(&stack, stackdir, &opts); - if (err < 0) - goto done; - - merged = reftable_stack_merged_table(stack); - reftable_table_from_merged_table(&table, merged); - err = reftable_table_print(&table); -done: - if (stack) - reftable_stack_destroy(stack); - return err; -} diff --git a/reftable/stack_test.c b/reftable/stack_test.c index dbca9eaf4a..42044ed8a3 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -179,13 +179,6 @@ static void test_reftable_stack_add_one(void) EXPECT(0 == strcmp("master", dest.value.symref)); EXPECT(st->readers_len > 0); - printf("testing print functionality:\n"); - err = reftable_stack_print_directory(dir, GIT_SHA1_FORMAT_ID); - EXPECT_ERR(err); - - err = reftable_stack_print_directory(dir, GIT_SHA256_FORMAT_ID); - EXPECT(err == REFTABLE_FORMAT_ERROR); - #ifndef GIT_WINDOWS_NATIVE strbuf_addstr(&scratch, dir); strbuf_addstr(&scratch, "/tables.list"); diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c index 19367c25f9..db62ea8dc3 100644 --- a/t/helper/test-reftable.c +++ b/t/helper/test-reftable.c @@ -1,6 +1,7 @@ #include "reftable/system.h" #include "reftable/reftable-error.h" #include "reftable/reftable-generic.h" +#include "reftable/reftable-merged.h" #include "reftable/reftable-reader.h" #include "reftable/reftable-stack.h" #include "reftable/reftable-tests.h" @@ -29,6 +30,26 @@ static void print_help(void) "\n"); } +static int dump_stack(const char *stackdir, uint32_t hash_id) +{ + struct reftable_stack *stack = NULL; + struct reftable_write_options opts = { .hash_id = hash_id }; + struct reftable_merged_table *merged = NULL; + struct reftable_table table = { NULL }; + + int err = reftable_new_stack(&stack, stackdir, &opts); + if (err < 0) + goto done; + + merged = reftable_stack_merged_table(stack); + reftable_table_from_merged_table(&table, merged); + err = reftable_table_print(&table); +done: + if (stack) + reftable_stack_destroy(stack); + return err; +} + static int dump_reftable(const char *tablename) { struct reftable_block_source src = { NULL }; @@ -87,7 +108,7 @@ int cmd__dump_reftable(int argc, const char **argv) } else if (opt_dump_table) { err = dump_reftable(arg); } else if (opt_dump_stack) { - err = reftable_stack_print_directory(arg, opt_hash_id); + err = dump_stack(arg, opt_hash_id); } if (err < 0) { -- cgit v1.3