ice_nine
ice_nine copied to clipboard
Replace Hash with Set in RecursionGuard.
While browsing IceNine's codebase I noticed that we can replace the Hash in RecursionGuard with a better fitting Set since we don't need the values.
@troessner I think there was a reason to use the object ids, but we'll have to wait for @dkubb to find the time to answer this.
I do not recall the details on why we track the object ids instead of the objects themselves. But I've got some thoughts on the approach.
Lets focus on the semantical difference: Tracking the object id, allows the object to be GCed. Turning the object id still in the recursion guard into an orphan.
This could lead to the memory allocator to allocate a new object that would resolve to the same id. While unlikely this seems still to be a lingering correctness issue that should be addressed, via tracking objects not their ids.
OT: I'd even go as far and assume that when somebody wants to freeze a cyclic, or "need to be GCed while frozen" datastructure a generic deep freeze like ice_nine
is the wrong thing to do.
I'd even support raising an exception on cycles.