[mclueppers]

The problem

BasicFileStore::iterator::load_value() reopens the segment file for every record it reads. operator* calls it at each position, so walking N records performs N opens of the same file.

On LittleFS that is the expensive half. An open() resolves the path from the root each time, and resolving a component replays that directory's metadata log via lfs_dir_fetchmatch — a log that grows with write activity until it is compacted. A store under steady write traffic therefore charges more and more per open while the record read itself stays the same size.

Anyone iterating a store on a device with a watchdog is affected, and the failure only appears once the store has been in service a while — a freshly created one is fast. Measured on a Heltec V4 (ESP32-S3) with 200 records on LittleFS: 162 ms per record, ~32 s for the full walk. The caller fed a 30 s task watchdog once per pass, so the node rebooted every ~90 seconds until this was found. Full report in #7.

To reproduce: put ~200 records in a BasicFileStore on LittleFS, let it take normal write traffic, then time for (auto it = store.begin(); it != store.end(); ++it) { auto& e = *it; }. Timing a single *it shows the cost is in open(), not the read.

The solution

Keep the segment handle in the iterator and reuse it, swapping only when the walk crosses a segment boundary:

if (!seg_file_ || seg_held_ != iv_.segment) {
    char name[USTORE_MAX_FILENAME_LEN];
    store_->segment_name(iv_.segment, name);
    seg_file_ = store_->_filesystem.open(name, File::ModeRead);
    seg_held_ = iv_.segment;
}

Why the iterator and not the store. A handle cached on the store would need invalidating against every write, remove, rename and compaction. Scoped to the iterator there is nothing new to invalidate: begin() already flushes pending writes before iterating, and mutating the store already invalidates the iterator. The change claims validity for exactly the iteration's lifetime and no longer.

Why it is not closed in load_value(). File holds a shared_ptr<FileImpl> and FileImpl closes on destruction, so the last copy to go away closes it exactly once. Closing explicitly would shut the file under the copy operator++(int) returns, which may still be walking. Every record is addressed by an explicit seek(), so a handle shared with a copied iterator cannot be left on a stale position.

Trade-offs and limitations. An iterator now holds one file handle open for its lifetime, where before it held none between dereferences — so a caller that keeps an iterator alive for a long time also keeps a handle open. This is the same handle the previous code opened and closed on every access, so the peak is unchanged; only the duration differs. Backends where open() is cheap simply see a smaller win. Records are still read in index order, which is random access into the segment file; ordering the walk by offset would help further but is a larger change and not attempted here.

Compatibility. No public API, on-disk format or protocol change. Behaviour is identical; only the number of open() calls differs.

Testing

Measured on a Heltec V4 (ESP32-S3, 8 MB flash), Arduino core 3.3.11 / ESP-IDF 5.5.5, BasicFileStore on LittleFS, 200 records, reached through microReticulum's path table:

	before	after
per record	162 ms	32 ms
64-record pass	10368 ms	2073 ms
first walk after boot	12312 ms	2118 ms
full 200-record walk	~32 s	~6.4 s

Held steady over a 270 s run on a single boot; the watchdog reboots the change was chased for stopped.

Native test suite (pio test -e native), before and after, on the same machine:

before (master 0f28567):  74 test cases: 3 failed, 70 succeeded
after  (this branch):     74 test cases: 3 failed, 70 succeeded

The three failures are pre-existing on master and unrelated — test_file_store_compact_basic, test_dir_prefix_reload_index and test_dir_prefix_compact, all failing on future-timestamp sweeping. test_iterator and test_typed_store, which exercise this code, pass in both runs.

Not tested: nRF52 / RAK4631 and the ttgo-lora32-v21 target (no hardware to hand), and the FAT/SD and FlashFileSystem backends. The change is backend-agnostic — it removes open() calls and does not alter what is read or written — so a backend with a cheap open() sees a smaller improvement rather than different behaviour.

Closes #7

[attermann]

Thanks for tracking this down and for the detailed measurements. The per-record open cost on LittleFS is a nasty one to find, and keeping the handle scoped to the iterator is the right place for it.

A couple remarks:
Wording: the description says the handle is swapped "when the walk crosses a segment boundary". But the walk follows std::unordered_map order, which is effectively random, not segment order. With records spread over several segments, it reopens a file on most steps, and the gain shrinks as the segment count grows. The reported 5× speedup probably reflects a store sitting mostly in one segment. The fix is still worthwhile; just keep expectations realistic for fragmented stores.
Missing tests: no test walks across more than one segment, and none uses post-increment while reading values. Small tests for both would cover the reopen path and the shared handle.

Thanks again for the contribution.