Remove hash tables and bump chunk from the DRC collector #11175
Conversation
This removes the explicit `HashSet`s used to represent the over-approximated-stack-roots and precise-stack-roots sets and replaces them with an intrusive, singly-linked list and a mark bit in the object headers respectively. The new list implementation also subsumes the old bump chunk that sat in front of the old over-approximated-stack-roots hash set. This shaves off about 25% of the time it takes to run the test case in bytecodealliance#11141 for me locally. This also ended up being a nice simplification of the DRC collector, which in turn allowed us to further simplify the `GcHeap` trait, since we no longer ever need to GC before passing GC refs into Wasm. Fixes bytecodealliance#11162
898354f to
da36faf
Compare
| ); | ||
| self.kind &= VMGcKind::MASK; |
There was a problem hiding this comment.
This was such a silly bug but it made me feel like I was losing my mind 🤦
| stack-roots list and decrementing its ref count" | ||
| ); | ||
| next = header.next_over_approximated_stack_root(); | ||
| let prev_next = header.next_over_approximated_stack_root(); |
There was a problem hiding this comment.
I'm not sure I see why we load this value twice -- can we use next below when we patch the prev link?
There was a problem hiding this comment.
VMGcRef doesn't implement Clone since cloning a VMGcRef potentially needs to run barriers, so we have dedicated methods to clone with those barriers if necessary. However, inside the GC implementation that isn't really a concern so we have unchecked_copy for these cases that don't need barriers. That said, when the GC ref is wrapped in an Option like it is here, it can be a bit of a mouthful, and we would have to do next.as_ref().map(|r| r.unchecked_copy()). So I figured I'd just grab it from the header twice and let LLVM clean things up.
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
This removes the explicit
HashSets used to represent the over-approximated-stack-roots and precise-stack-roots sets and replaces them with an intrusive, singly-linked list and a mark bit in the object headers respectively. The new list implementation also subsumes the old bump chunk that sat in front of the old over-approximated-stack-roots hash set.This shaves off about 25% of the time it takes to run the test case in #11141 for me locally.
This also ended up being a nice simplification of the DRC collector, which in turn allowed us to further simplify the
GcHeaptrait, since we no longer ever need to GC before passing GC refs into Wasm.Fixes #11162