From dcd4095c3720b0f6025b9baa0cc60c76cebfd163 Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Sat, 5 Sep 2026 21:08:28 +0100 Subject: [PATCH] Reuse the segment handle across an iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The iterator reopened the segment file for every record. On LittleFS an open resolves the path afresh, and resolving a component replays that directory's metadata log, which grows with write activity until it is compacted — so a store that is appended to constantly charges more and more per open while the read itself stays the same size. Measured on a 200-record path store on an ESP32-S3, one record cost 93-162 ms, nearly all of it under lfs_dir_find. The caller walking the table spent 32 s doing so and its task watchdog fired. The handle is held for the iteration and swapped when a walk crosses a segment boundary. begin() already flushes pending writes before iterating and mutating the store already invalidates the iterator, so nothing new is claimed about validity. It is deliberately not closed in load_value(): File holds a shared_ptr and FileImpl closes on destruction, so the last copy closes it exactly once, whereas an explicit close would shut the file under the copy operator++(int) returns. --- include/microStore/FileStore.h | 40 +++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/include/microStore/FileStore.h b/include/microStore/FileStore.h index c243e21..f58c65c 100644 --- a/include/microStore/FileStore.h +++ b/include/microStore/FileStore.h @@ -843,13 +843,37 @@ USTORE_LOG("[ustore] set_max_recs: %u\n", max_recs); // Read the value from disk into current_.value. Called lazily by // operator* / operator->. Marked const so it can mutate mutable members. + // + // The segment file is held open across the walk rather than reopened + // per record. Opening is not the cheap half: on LittleFS every open + // resolves the path afresh, and resolving a component replays that + // directory's metadata log, which grows with write activity until it + // is compacted. A store that is appended to constantly therefore + // charges more and more for each open while the read itself stays the + // same size. Measured on a 200-record path store on an ESP32-S3, one + // record cost 93-162 ms, nearly all of it under lfs_dir_find — the + // caller walking the table spent 32 s doing so and its task watchdog + // fired. + // + // Held for exactly the iteration's lifetime, which is all this claims: + // begin() flushes pending writes before iterating, so what is on disk + // is current, and mutating the store already invalidates the iterator. + // Every record is addressed by an explicit seek, so a handle shared + // with a copied iterator cannot be left on a stale position. It is not + // closed here on purpose — File holds a shared_ptr and FileImpl closes + // on destruction, so the last copy to go away closes it exactly once; + // closing it explicitly would shut the file under a copy (the one + // operator++(int) returns) that may still be walking. void load_value() const { - char name[USTORE_MAX_FILENAME_LEN]; - store_->segment_name(iv_.segment, name); - - File f = store_->_filesystem.open(name, File::ModeRead); + 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; + } value_loaded_ = true; - if (!f) { current_.value.clear(); return; } + if (!seg_file_) { current_.value.clear(); return; } + File& f = seg_file_; f.seek((long)iv_.offset, SeekModeSet); RecordHeader hdr; @@ -858,7 +882,6 @@ USTORE_LOG("[ustore] set_max_recs: %u\n", max_recs); current_.value.resize(hdr.length); if (hdr.length > 0) f.read(current_.value.data(), hdr.length); - f.close(); } BasicFileStore* store_; @@ -867,6 +890,11 @@ USTORE_LOG("[ustore] set_max_recs: %u\n", max_recs); IndexValue iv_; mutable bool value_loaded_; mutable Entry current_; + // The segment this iterator currently holds open, and which one it is. + // A store spans several segments, so a walk that crosses a boundary + // swaps the handle rather than keeping one per segment open. + mutable File seg_file_; + mutable uint32_t seg_held_ = 0; }; /* -------- BEGIN / END -------- */