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

Fix compaction on nodes (backport to v1.13.x) #2588

Merged
merged 7 commits into from
Jul 12, 2022

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented Jul 10, 2022

What problem is this PR intended to solve?

Backport of #2579 to v1.13.x

  • don't merge without squashing. this draft PR is to get feedback from CI.

@flavorjones flavorjones added the backport Backport of a PR to the current release branch label Jul 10, 2022
@flavorjones flavorjones added this to the v1.13.x patch releases milestone Jul 10, 2022
@flavorjones flavorjones changed the title bFix compaction on nodes (backport to v1.13.x) Fix compaction on nodes (backport to v1.13.x) Jul 10, 2022
@flavorjones flavorjones added the topic/memory Segfaults, memory leaks, valgrind testing, etc. label Jul 10, 2022
@flavorjones
Copy link
Member Author

Will look at CI failures tomorrow.

@flavorjones flavorjones force-pushed the fix-compaction-on-nodes_backport-to-v1.13.x branch from 7cdd4ee to 3993f1e Compare July 11, 2022 15:54
@flavorjones flavorjones marked this pull request as ready for review July 11, 2022 15:55
tenderlove and others added 6 commits July 11, 2022 12:59
Unconditionally set a mark function on the node wrapper.  We will later
refactor this to use TypedData_Wrap_Struct and we don't want to
conditionally set mark functions on that

Co-Authored-By: Mike Dalessio <mike.dalessio@gmail.com>
This commit introduces a `Noko_Node_Get_Struct` macro for unwrapping
the underlying data pointer from Nokogiri objects. Then we change all
places that unwrap nodes to use the new macro. We also converted
xmlNode to use `TypedData_Wrap_Struct` so we can provide a compaction
function.

Note that we use DATA_PTR instead of "typed" unwrappers everywhere
because the large number of C structs that inherit from xmlNode would
make the unwrapping code unwieldy and type-check-heavy.

Co-Authored-By: Matt Valentine-House <matt@eightbitraptor.com>
Co-Authored-By: Mike Dalessio <mike.dalessio@gmail.com>
This fixes the case where nodes moves and we need to update the private
references.

Co-Authored-By: Matt Valentine-House <matt@eightbitraptor.com>
Co-Authored-By: Mike Dalessio <mike.dalessio@gmail.com>
Co-Authored-By: Mike Dalessio <mike.dalessio@gmail.com>
We don't have `rb_gc_location` everywhere, so check for it in the
extconf and then conditionally set the compaction callback

Co-Authored-By: Mike Dalessio <mike.dalessio@gmail.com>
for warnings triggered by the compaction test.

- for memory allocated in ary_heap_realloc
- for memory added to the mark stack in stack_chunk_alloc
@flavorjones flavorjones force-pushed the fix-compaction-on-nodes_backport-to-v1.13.x branch from 3993f1e to 5f63c8b Compare July 11, 2022 17:00
and make sure the OCI image used in CI matches
@flavorjones flavorjones merged commit a13241e into v1.13.x Jul 12, 2022
@flavorjones flavorjones deleted the fix-compaction-on-nodes_backport-to-v1.13.x branch July 12, 2022 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Backport of a PR to the current release branch topic/memory Segfaults, memory leaks, valgrind testing, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants