Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimization: consider alternative document free mechanisms that will allow setting RUBY_TYPED_FREE_IMMEDIATELY #2822

Open
flavorjones opened this issue Mar 7, 2023 · 1 comment
Labels
topic/memory Segfaults, memory leaks, valgrind testing, etc.
Milestone

Comments

@flavorjones
Copy link
Member

See 0804380 for more context, but essentially:

  • we parent unparented nodes at the last minute so that xmlFreeDoc frees all the underlying memory for us
  • if two text nodes happen to end up siblings during that process, they are merged and libxml2 may allocate additional memory via ruby_xmalloc
  • it may be unsafe to allocate memory during GC, warnings will be raised

Working on #2807 made me realize that we're doing more work during the deallocate function than I thought -- so it's both slower and less safe than it could be.

I have at least one idea, which is instead of parenting the unparented nodes, just call xmlFreeNode on them to avoid the parenting overhead.

@flavorjones flavorjones added the topic/memory Segfaults, memory leaks, valgrind testing, etc. label Mar 7, 2023
@byroot
Copy link
Contributor

byroot commented Mar 7, 2023

just call xmlFreeNode on them to avoid the parenting overhead.

I tried that and got some crashes because the pointers weren't allocated, not too shure what kind of shenanigans libxml does internally, but I suspect it might have some kind of arenas.

@flavorjones flavorjones added this to the v1.18.0 milestone Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/memory Segfaults, memory leaks, valgrind testing, etc.
Projects
None yet
Development

No branches or pull requests

2 participants