Skip to content

Bug: TypedStore's iterator reads the record on every step, so advancing costs as much as dereferencing #9

Description

@mclueppers

Version / platform

  • microStore 0.1.7 (master, 0f28567)
  • Reproduces on the host: pio test -e native, using the RAM filesystem the typed-store tests already have
  • Also observed on hardware — Heltec/LilyGO ESP32 boards, Arduino core 3.3.11 / ESP-IDF 5.5.5, BasicFileStore on LittleFS, reached through microReticulum 0.5.x

Expected behaviour

BasicFileStore is lazy by design: operator* loads the value from disk on first dereference and operator-> is documented as metadata-only, "without touching disk". A caller that walks a store and dereferences only some entries should pay only for the ones it reads.

Actual behaviour

TypedStore::iterator decodes the record in its constructor and again in operator++:

iterator(typename Store::iterator it, typename Store::iterator end)
    : it_(std::move(it)), end_(std::move(end))
{
    if (it_ != end_) load();
}

iterator& operator++()
{
    ++it_;
    if (it_ != end_) load();
    return *this;
}

void load()
{
    const auto& raw = *it_;  // operator* triggers lazy value load
    ...
}

load() dereferences the underlying iterator, so the lazy read the file store is careful to defer happens on every step. The typed wrapper undoes its own store's laziness one layer up.

Counted through the RAM filesystem the typed-store tests use, stepping over 8 records without dereferencing any of them performs 8 opens:

test_typed_store_iterator_advance_reads_nothing: Expected 0 Was 8   [FAILED]

Impact

Anything that does not dereference every entry pays for entries it never looks at: counting, searching for a key, or stopping early all read the whole prefix.

The sharper consequence is that reaching position N always costs N reads, which rules out bounded or resumable traversal. That matters on a device where a record read is expensive: on an ESP32 with the store on LittleFS we measure roughly 32 ms per record. We tried to make a long table walk watchdog-safe by giving each pass a time budget and resuming from a saved position on the next one. It cannot work — the cursor cannot skip to where it left off any more cheaply than re-reading everything before it, so the walk is O(n²) and, once the prefix alone exceeds the budget, stops advancing at all. On a node holding 87 paths it sat at position 15 for ever, re-reading the same 15 records every pass and never reaching the rest.

Steps to reproduce

pio test -e native with this test added to test/test_typed_store (it uses the RAM filesystem already in that file, plus a counter on its open()):

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);      // fails: 8

Suggested fix

Defer the decode to operator* behind a "already loaded at this position" flag, so operator++ only advances and a second dereference at one position still reads once. A walk that dereferences every entry then costs exactly what it does today, and one that skips entries costs nothing for the ones it skips. No API change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions