verifying parent nodes' existence #2488
Conversation
| @@ -84,7 +84,10 @@ impl GraphMemoryManagement { | |||
| for (id, parents) in self.nodes.iter() { | |||
| let is_useful = matches!(self.statuses.get(id), Some(NodeMemoryStatus::Useful)); | |||
|
|
|||
| if !is_useful && Arc::strong_count(id) == 1 && parents.is_empty() { | |||
| let parents_absent = | |||
| parents.is_empty() || parents.iter().all(|p| !self.nodes.contains_key(p)); | |||
There was a problem hiding this comment.
If parents.is_empty() == true, parents.iter().all(...) == true. If the second expression works, the first expression i.e. is_empty has no effect.
https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.all
There was a problem hiding this comment.
I initially kept parents.is_empty() to reflect the intended logic, but you are absolutely correct that it has no effect here. I’ll go ahead and make that adjustment. Thanks again!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2488 +/- ##
=======================================
Coverage 82.91% 82.91%
=======================================
Files 812 812
Lines 105334 105336 +2
=======================================
+ Hits 87336 87338 +2
Misses 17998 17998 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Hi, I used my project as "my" example program to test your changes.
I used Apple's Instruments.app to profile heap memory every 10 seconds, below are the results:
It indicates that your changes work for me.
I think the moderators can merge it.
Miscellaneous
As for MNIST example, I don't know why I can't run it with:
./target/debug/examples/mnistIt looks like I messed up $DYLD_LIBRARY_PATH again.
There was a problem hiding this comment.
Thanks for both flagging the issue and fixing it 🎉
@AsherJingkongChen thank you also for validating
Fix #2487
Since the loop over self.nodes.iter() in GraphMemoryManagement::clear_unused_roots() does not process nodes in child-to-parent order, if a node's parents are removed first, that node will not be deleted and will remain. To address this, I added a check to verify if all parents are absent from self.nodes. If none of the parents exist in self.nodes, the node is treated as having no parents and is marked for deletion.