internal: remove mark function from XML::Document #2926
Conversation
| rb_gc_mark(tuple->doc); | ||
| rb_gc_mark(tuple->node_cache); |
There was a problem hiding this comment.
So if I understand correctly, these marks aren't necessary to prevent the objects being GCed, because one is a reference to self, and another is held some other way.
However they were added so the objects would be pinned.
Up until this point it checks out, but AFAIK we didn't add any dcompact functions, did we? At least not in the commit you linked. So we either need to add a compaction callback to update the references, or we need to keep them pined.
There was a problem hiding this comment.
@byroot That's correct, they were being marked so that they wouldn't get relocated.
🤔 Ahh, you're right, we do need to implement dcompact, my bad.
There was a problem hiding this comment.
Although: the reason this doesn't segfault now is, I think, because a Node calls rb_gc_mark on its Document owner, essentially keeping it pinned anyway.
What problem is this PR intended to solve?
remove mark function from XML::Document. originally introduced in dd07ce8, I don't think we need it anymore since cb1557d introduced TypedData conventions to support compaction.
cc @tenderlove and @byroot for a gut check
Have you included adequate test coverage?
existing coverage should be sufficient to detect if there's an issue with compaction.
Does this change affect the behavior of either the C or the Java implementations?
No behavior changes.