Ensure minimal number of GraphArrayView invalidation calls#300
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #300 +/- ##
==========================================
+ Coverage 87.81% 87.89% +0.07%
==========================================
Files 57 57
Lines 4998 4997 -1
Branches 877 876 -1
==========================================
+ Hits 4389 4392 +3
+ Misses 384 382 -2
+ Partials 225 223 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Make _invalidate_bbox accept parallel sequences of times and bboxes so the node event handlers issue a single call instead of one per node. The cache's invalidate() is left untouched: invalidation only flips ready flags and zeroes buffer regions, and recomputation is already coalesced in NDChunkCache.get(), so grouping there buys nothing. Read the bbox via dict.get() and treat a missing bbox as an unknown location, invalidating the whole volume for that time. This keeps it distinct from a bbox that lies outside the array volume (which invalidates nothing) and prevents a KeyError from the cache observer when add_node/bulk_add_nodes emit attrs without a bbox key.
|
@cmalinmayor, I merged with main and made it work with a list of items. What about if the masks are changed? |
The node-updated handler only invalidated when the displayed attribute value or the bbox changed, so swapping a node's mask while keeping the same bbox left a stale rendering in the cache. Add a mask-content check (_mask_changed) so an unchanged-bbox mask update still invalidates the region, while a re-supplied identical mask invalidates nothing.
JoOkuma
left a comment
JoOkuma
left a comment
There was a problem hiding this comment.
@cmalinmayor LGTM, I made quite a few changes.
With the recent changes, it takes a list of events.
Could you take a look to see if you agree with everything, or if there's anything else to add?
|
@cmalinmayor, I'm addressed your comment. I'm going to merge this. If I missed anything, we can open another PR. |
3 participants
Currently, GraphArrayView invalidates the old and new bboxes in the cache any time any node attribute is updated, including totally unrelated ones.
Also, it appears that SQLGraph emits the signal that triggers this twice:
tracksdata/src/tracksdata/graph/_sql_graph.py
Lines 2110 to 2111 in 13f9ba8
tracksdata/src/tracksdata/graph/_sql_graph.py
Lines 2119 to 2120 in 13f9ba8
Every cache invalidation is expensive, and re-invaliding the same area multiple times resets any progress already made in loading.
This PR ensures the following behavior for all graph backends: