From a2134139711ebdbe157cc3dce3037e4fcee46186 Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Sun, 6 Sep 2026 13:41:01 +0100 Subject: [PATCH] Load the record on dereference, not on every step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TypedStore's iterator decoded the record in its constructor and again on every ++, so advancing read it off the filesystem whether the caller wanted it or not. BasicFileStore is lazy on purpose — it reads only on operator*, and documents operator-> as metadata-only for that reason — and the typed wrapper took that back one layer up. The cost is paid by anything that does not dereference every entry: counting, searching for a key, or stopping early all read the whole prefix, and reaching position N always costs N reads. That last one rules out bounded or resumable traversal, because a cursor cannot skip to where it left off more cheaply than re-reading everything before it. Decoding now happens in operator*, guarded so the end iterator is still never dereferenced, with a flag so asking twice at one position reads once. A full walk that dereferences every entry costs exactly what it did before. Adds test_typed_store_iterator_advance_reads_nothing, which counts opens through the RAM filesystem the typed-store tests already use: stepping over 8 records without dereferencing did 8 opens before this change and does 0 after. --- include/microStore/TypedStore.h | 23 +++++++-- test/test_typed_store/test_typed_store.cpp | 57 ++++++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/include/microStore/TypedStore.h b/include/microStore/TypedStore.h index 1f15a50..a6ceecc 100644 --- a/include/microStore/TypedStore.h +++ b/include/microStore/TypedStore.h @@ -93,16 +93,24 @@ class TypedStore { public: + // Decoding is deferred to operator*, so advancing costs nothing. + // + // The store underneath is lazy on purpose: BasicFileStore reads a + // record only when it is dereferenced, and its operator-> is + // documented as metadata-only for exactly that reason. Loading in the + // constructor and in operator++ took that back — every step read the + // record off the filesystem whether the caller wanted it or not, so a + // traversal paid for entries it never looked at and reaching position + // N always cost N reads. iterator(typename Store::iterator it, typename Store::iterator end) - : it_(std::move(it)), end_(std::move(end)) + : it_(std::move(it)), end_(std::move(end)), loaded_(false) { - if (it_ != end_) load(); } iterator& operator++() { ++it_; - if (it_ != end_) load(); + loaded_ = false; return *this; } @@ -113,6 +121,7 @@ class TypedStore Entry& operator*() { + if (!loaded_) load(); return current_; } @@ -121,10 +130,16 @@ class TypedStore typename Store::iterator it_; typename Store::iterator end_; Entry current_; + bool loaded_; void load() { - const auto& raw = *it_; // operator* triggers lazy value load + loaded_ = true; + // The end iterator is not dereferenceable. The constructor used to + // say so by not loading; the guard moves here now that the load + // happens on demand. + if (it_ == end_) return; + const auto& raw = *it_; // operator* triggers the store's lazy value load KeyCodec::decode(raw.key, current_.key); ValueCodec::decode(raw.value, current_.value); } diff --git a/test/test_typed_store/test_typed_store.cpp b/test/test_typed_store/test_typed_store.cpp index ba0784e..cfd4221 100644 --- a/test/test_typed_store/test_typed_store.cpp +++ b/test/test_typed_store/test_typed_store.cpp @@ -123,9 +123,14 @@ class RamFileImpl : public microStore::FileImpl { virtual bool isValid() const override { return !_closed; } }; +// Counts opens so a test can assert what a traversal actually reads, rather +// than assert that it is fast and hope. +static int g_opens = 0; + class RamFileSystemImpl : public microStore::FileSystemImpl { protected: virtual microStore::File open(const char* path, microStore::File::Mode mode, const bool create = false) override { + g_opens++; bool wr = (mode == microStore::File::ModeWrite || mode == microStore::File::ModeReadWrite); bool ap = (mode == microStore::File::ModeAppend || mode == microStore::File::ModeReadAppend); int idx = find_file(path); @@ -173,6 +178,7 @@ class RamFileSystemImpl : public microStore::FileSystemImpl { static void reset_ram_fs() { for (int i = 0; i < g_nfiles; i++) { g_files[i].data.clear(); g_files[i].pos = 0; g_files[i].open = false; } g_nfiles = 0; + g_opens = 0; } static microStore::FileSystem make_ram_fs() { return microStore::FileSystem{new RamFileSystemImpl()}; } @@ -336,6 +342,56 @@ void test_typed_store_iterator() { } // TypedStore> round-trips raw binary values. +void test_typed_store_iterator_advance_reads_nothing() { + reset_ram_fs(); + + { + microStore::FileStore fs_store; + auto fs = make_ram_fs(); + fs_store.init(fs, "/lazy"); + StringStore writer(fs_store); + for (int n = 0; n < 8; n++) { + writer.put(std::string("k") + (char)('0' + n), std::string("v") + (char)('0' + n)); + } + } + remove_ram_file("/lazy_index.dat"); + + microStore::FileStore fs_store2; + auto fs2 = make_ram_fs(); + fs_store2.init(fs2, "/lazy"); + StringStore reader(fs_store2); + + // Walking without dereferencing must not touch the filesystem: the store + // underneath reads a record only on operator*, and the typed wrapper is + // not allowed to undo that. Without this, a caller that skips entries — + // or merely counts them — pays a read for every one. + g_opens = 0; + int stepped = 0; + for (auto it = reader.begin(); it != reader.end(); ++it) stepped++; + TEST_ASSERT_EQUAL(8, stepped); + TEST_ASSERT_EQUAL(0, g_opens); + + // ...and the values are still there for a caller that does ask. + g_opens = 0; + int read = 0; + for (auto it = reader.begin(); it != reader.end(); ++it) { + auto& e = *it; + TEST_ASSERT_EQUAL(2, (int)e.key.size()); + TEST_ASSERT_EQUAL(2, (int)e.value.size()); + read++; + } + TEST_ASSERT_EQUAL(8, read); + TEST_ASSERT_TRUE(g_opens > 0); + + // Dereferencing twice at one position reads once. + auto it = reader.begin(); + g_opens = 0; + (void)*it; + const int first = g_opens; + (void)*it; + TEST_ASSERT_EQUAL(first, g_opens); +} + void test_typed_store_vector_value() { reset_ram_fs(); @@ -453,6 +509,7 @@ int runUnityTests(void) { RUN_TEST(test_typed_store_exists); RUN_TEST(test_typed_store_size); RUN_TEST(test_typed_store_iterator); + RUN_TEST(test_typed_store_iterator_advance_reads_nothing); RUN_TEST(test_typed_store_vector_value); RUN_TEST(test_typed_store_keyset); RUN_TEST(test_typed_store_custom_int_codec);