Persistent managed memmap caches + preload="memmap" sentinel - #14216
Conversation
eaa77b9 to
36ccfc5
Compare
|
Okay for this one... I'm thinking the real fix is to fix |
36ccfc5 to
5e2edee
Compare
|
hey @larsoner, bonjour! This is my proposal to really speed up! I completely pivot from the initial trick, which I plan to address in another PR. This way, we can have both worlds, nothing is breaking, and we consume the. _ data object more seriously, only changing one flag, the preload="auto". I will still iterate this across the day, but I feel that this will unlock a lot of power with this base memmap representation. It will not be used without the person knowing the flag preload="auto" |
5e2edee to
84aae35
Compare
larsoner
left a comment
There was a problem hiding this comment.
I agree a preload="auto" could have some benefits, but the implementation is much too big to be maintainable.
Would hashing on the filename + mtime of the file be enough, rather than needing to triage based on internal file stuff?
If not, is there some simpler idea that could work?
|
@larsoner, could you please take another look at a138fb7, especially at whether this is now a maintainable/minimal direction? I followed your simplification suggestion quite aggressively:
I could not safely use only One other design question: the simplification treats the configured MNE cache directory as trusted/private and consequently removed the previous regular-file and symlink validation. Is that the contract you would expect, or should I retain a small explicit validation check? Here is the complete microbenchmark I used (run from the PR checkout with an empty # python bench.py RECORDING CACHE_DIR
import os
import sys
from pathlib import Path
from time import perf_counter
import mne
fname, cache_dir = map(Path, sys.argv[1:3])
cache_dir.mkdir(parents=True, exist_ok=True)
os.environ["MNE_CACHE_DIR"] = str(cache_dir)
print(mne.__file__)
for label in ("miss", "hit"):
start = perf_counter()
raw = mne.io.read_raw(fname, preload="auto", verbose="error")
opened = perf_counter()
raw.get_data(stop=min(1024, raw.n_times))
accessed = perf_counter()
print(
f"{label}: open={(opened - start) * 1e3:.1f} ms, "
f"1024 samples={(accessed - opened) * 1e3:.3f} ms"
)I executed this exact snippet against For the separate interleaved 900 MiB format benchmark, median warm opens were 16.1 ms (FIF), 14.6 ms (EDF), 15.7 ms (BDF), and 24.3 ms (BrainVision), with matching decoded-data checksums. |
|
Windows failure looks real: |
larsoner
left a comment
There was a problem hiding this comment.
Approach seems much simpler, thanks! Now I'm curious to see if we can generalize it. Moving it to baseraw will require looking to see if a fname-like arg is passed and if it's not file-like, but it should buy us support for this for almost all formats (and any new ones).
And thus some smoke test for it would ideally maybe be added to the def _test_raw function, which is run for every I/O format
| @@ -0,0 +1,2 @@ | |||
| Speed up repeated preloading of FIF, EDF/BDF, and BrainVision | |||
There was a problem hiding this comment.
Why just these three? Can't we build this into the BaseRaw class like we already do for strings?
Publish cache entries through a per-process temporary and os.replace instead of a FileLock, so filelock stays an optional dependency rather than a required one. Make load_data(memmap="auto") resolve the same sentinel as preload="auto"; previously it meant a file literally named "auto". Stop reusing a live-mapped filename in test_auto_preload_api, which Windows rejects when truncating a file that still has a mapping.
|
@larsoner CI is green now, Windows included. The Windows failure was in the test, not the cache: it created a Raw with Two other things while I was in there:
On your changelog question: the cache isn't reader-specific, |
Move the per-format preload="auto" coverage into _test_raw_reader so every reader with test_preloading=True exercises the decoded-data cache, stat directory sources (e.g. CTF .ds) member-wise, and make RawCurry honor a non-bool preload instead of silently ignoring it.
Teach the generic reader test about the binfile keyword and run it for read_raw_fil, which also exercises preload="auto" for that format.
RawANT._read_segment_file assigned every channel into the output buffer, which fails whenever a projector shrinks it, so route the chunk through _mult_cal_one and cover the reader with _test_raw_reader.
larsoner
left a comment
There was a problem hiding this comment.
Awesome, thanks @bruAristimunha !
Four review passes (reuse, simplification, efficiency, altitude) against the shape of mne-tools#14216. Measured rather than assumed: - byte-budget heuristic: never fires. n_per already caps a chunk near 10 MiB of source bytes, so temporaries stay under 50 MB of the 64 MB cap even for an adversarial 2-channel/6-hour file with reversed picks. - n_read == 1 branch: 0/200 hits on the windowed benchmark, and the general branch produces the same values. - threading: real (13-22% on preload) but reachable only via direct_output, and it is one of only two ThreadPoolExecutor sites in MNE. mne.parallel already has parallel_func(prefer='threads'). Deferred to its own PR with a benchmark. - direct_output: kept, it measures 27-28% on full preload. _read_segments_file mmap/threading and the BrainVision block sizing are reverted to main and move to a follow-up: no MNE fixture is large enough to reach the 64 MB threading threshold (largest .eeg is 3.7 MB), so it was untested at any realistic scale. Tests 549 -> 156 lines: four near-identical stride tests merged into one parametrized test, redundant file_kind axes dropped, and a test asserting x * 1.0 == x bit-exactly removed. +884/-29 -> +330/-18. Output stays bit-identical to main: 77/77 array snapshots and 56/56 annotation snapshots across 28 files.
Four review passes (reuse, simplification, efficiency, altitude) against the shape of mne-tools#14216. Measured rather than assumed: - byte-budget heuristic: never fires. n_per already caps a chunk near 10 MiB of source bytes, so temporaries stay under 50 MB of the 64 MB cap even for an adversarial 2-channel/6-hour file with reversed picks. - n_read == 1 branch: 0/200 hits on the windowed benchmark, and the general branch produces the same values. - threading: real (13-22% on preload) but reachable only via direct_output, and it is one of only two ThreadPoolExecutor sites in MNE. mne.parallel already has parallel_func(prefer='threads'). Deferred to its own PR with a benchmark. - direct_output: kept, it measures 27-28% on full preload. _read_segments_file mmap/threading and the BrainVision block sizing are reverted to main and move to a follow-up: no MNE fixture is large enough to reach the 64 MB threading threshold (largest .eeg is 3.7 MB), so it was untested at any realistic scale. Tests 549 -> 156 lines: four near-identical stride tests merged into one parametrized test, redundant file_kind axes dropped, and a test asserting x * 1.0 == x bit-exactly removed. +884/-29 -> +330/-18. Output stays bit-identical to main: 77/77 array snapshots and 56/56 annotation snapshots across 28 files.
Reference issue (if any)
None. Part 5/5 of the IO-speedup series. Depends on #14215
(→ #14214 → #14213 → #14212).
What does this implement/fix?
preload="memmap"on any reader: selects an automatically managedmemory-map cache keyed by source path/mtime/size. First read decodes into
it; later reads, including from fresh processes, mmap it directly.
load_data(memmap=<path>)caches are reused when valid instead of alwaysre-decoding.
BaseRaw.__del__no longer deletes memmap-backed files; cache lifetime isowned by the caller. Behavior change, noted in the changelog fragment.
Fresh-process open + first access of a 944 MB float64 recording:
~450 ms decode → 11–21 ms. Public-API windows: ~84 µs.
Additional information
AI disclosure: same as #14212.
CI note: held until the parent PR merges.