Repository navigation
Conversation
Every ChannelmapGUI session called backend.make_wiring_maps(), which unpickled its own ~24 MB copy of the electrode wiring conflict maps. The result is identical across sessions and only ever read, so make_wiring_maps now resolves its argument and delegates to an lru_cache-backed loader. Every caller for a given directory, whether it passes a Path or a str, gets the same object. Split out of m-beau#52. Assisted-by: ClaudeCode:claude-opus-5-5
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
=======================================
Coverage ? 70.17%
=======================================
Files ? 20
Lines ? 3128
Branches ? 0
=======================================
Hits ? 2195
Misses ? 933
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lru_cache(maxsize=None) is what functools.cache already spells. Assisted-by: ClaudeCode:claude-opus-5-5
make_wiring_maps already asserts that the resolved directory exists before calling _load_wiring_maps, so the loader's own check never fires. Assisted-by: ClaudeCode:claude-opus-5-5
The sharing tests assert object identity, so a map cached by an earlier test (or left behind by one) could make them pass or fail depending on test order. Assisted-by: ClaudeCode:claude-opus-5-5
make_wiring_maps now returns one object shared by every session, so an in-place mutation (e.g. |= or .add() on a conflict set) would silently change electrode availability for everyone. Return MappingProxyType views with frozenset values instead, so mutating raises an error. The pickle on disk stays plain dicts and sets. Load time and peak RSS are unchanged. The Electrodes annotation now accepts any read-only mapping. Assisted-by: ClaudeCode:claude-opus-5-5
Keep the contract (cached per directory, shared, read-only) and leave the measurements and history to the commit log and PR. The read-only rule no longer needs spelling out now that the mapping enforces it. Assisted-by: ClaudeCode:claude-opus-5-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖
Split out of #52 (the performance half).
What
make_wiring_maps(wiring_maps_dir)resolves its argument and passes it to_load_wiring_maps, a loader wrapped infunctools.cache. Each process holds one copy of the wiring conflict maps per directory, and every caller gets that same object whether it passes aPathor astr.MappingProxyTypeviews withfrozensetvalues. A stray in-place mutation now raises an error instead of quietly changing electrode availability for every session. The pickle on disk is still plain dicts and sets.Electrodes.wiring_mapannotation now accepts any read-only mapping.Why
ChannelmapGUI.__init__callsmake_wiring_mapsonce per Panel session, and each call unpickled its own ~24 MB copy. Every copy is identical, and the GUI only reads it:Electrodescopies.keys(), looks upwiring_map[e]and does&intersections, all of which build new sets. That makes sharing one copy safe. Freezing it doesn't measurably change load time (0.24s vs 0.23s) or peak RSS (+70 MB either way).Commits
Each commit does one thing:
functools.cache.Each commit passes the tests on its own.
Tests
TestWiringMapsSharingcovers:Pathvsstrand across callscache_clear()reloadsElectrodesselect/deselect/undo/redo on the shared map leaves it unchanged and doesn't affect another sessionuv run pytest -q: 572 passed, 1 xfailed.Assisted-by: ClaudeCode:claude-opus-5-5