Add safeguards to activateRoots and optimize render ops #41
Conversation
Fix gc crash
Fix gc crash
…into fix-gc-crash
* optimize: remove hash table lookup from render ops * bump
don't crossfade in first graph render sequence
runtime/elem/Runtime.h
Outdated
| } | ||
| } | ||
| } | ||
|
|
||
| // Merge | ||
| currentRoots = active; | ||
| currentRoots.swap(active); // More efficient than assignment |
There was a problem hiding this comment.
Oh really? I'm curious to learn about that, mind explaining?
There was a problem hiding this comment.
I think for the std::set it might be a bit more efficient than the assignment because it only deals with internal pointers
There was a problem hiding this comment.
I think maybe the most clear would be currentRoots = std::move(active); ? would do no copy like the call to swap but more clarity.
though, i really don't think this is a bottleneck at all. and I imagine a compiler might pick up the move call internally anyways since its at the bottom of the function
either way, active is deallocated at the end of the function call. if you really wanted to optimize with swap, active could be a member?
but imo whatever makes the code clearest here is the best. = seems fine. this function isn't called in a super tight loop or anything
thoughts @cannc4 ?
There was a problem hiding this comment.
Agreed @ncblair! I see, yea so swap skips the copy whereas otherwise we were using a copy-assignment, got it! I'm ok with either swap or currentRoots = std::move(active)
It should fine to merge this one as @ncblair's changes are also included here but up to you |
Changes