Add cache clearing to stop the memory leak - #1106
Conversation
Deploying with
|
| Latest commit: |
6fff1ef
|
| Status: | ⚡️ Build in progress... |
3c5abf7 to
35feab2
Compare
| impl<T> LetNode<T> { | ||
| pub fn new() -> LetNode<T> { | ||
| LetNode { cache: boxcar::Vec::new() } |
There was a problem hiding this comment.
This is actually the most relevant part, we can basically replace the box car with a single unsafe cell because this should during the runtime of the graph only ever have one value which can be thrown away at the end
There was a problem hiding this comment.
Nevermind didn't see this was already implemented for the cache node
TrueDoctor
left a comment
TrueDoctor
left a comment
There was a problem hiding this comment.
We should ideally let miri run on a test that evaluates the nodegraph multiple times to check if this contains any subtle UB but this looks fine for now
| pub fn eval_any<'i>(&'i self, id: NodeId, input: Any<'i>) -> Option<Any<'i>> { | ||
| let node = self.nodes.get(&id)?; | ||
| Some(node.node.eval(input)) | ||
| Some(unsafe { (*((&*node.read().unwrap()) as *const NodeContainer)).node.eval(input) }) |
There was a problem hiding this comment.
Oof I dislike the evaluation being an unsafe operation but I guess its fine because it only is a lifetime extension
There was a problem hiding this comment.
That is a big ugly - I wasn't quite sure how to resolve this though.
da2292b to
6fff1ef
Compare
* Add cache clearing * Add TODO comment --------- Co-authored-by: Keavon Chambers <keavon@keavon.com>
3 participants
Clears the node graph cache between runs, leading to reduced memory usage.
Closes #1103