-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Find roots when running GC rather than runtime #3109
Conversation
Attempt to address the issue boa-dev#2773. The existing implementation had an expensive overhead of managing root counts, especially for mutable borrow of GcRefCell. Instead of managing the root counts, this change counts the number of Gc/WeakGc handles located in Gc heap objects and total number of them. Then, we can find whether there is a root by comparing those numbers.
Codecov Report
@@ Coverage Diff @@
## main #3109 +/- ##
==========================================
- Coverage 50.61% 50.58% -0.03%
==========================================
Files 444 443 -1
Lines 42696 42641 -55
==========================================
- Hits 21612 21572 -40
+ Misses 21084 21069 -15
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really liking the big performance wins! However, I have some suggestions about the design.
There are too many |
boa_gc/src/internals/gc_box.rs
Outdated
/// - Mark Flag Bit | ||
/// | ||
/// The next node is set by the `Allocator` during initialization and by the | ||
/// `Collector` during the sweep phase. | ||
pub(crate) struct GcBoxHeader { | ||
roots: Cell<usize>, | ||
marked: Cell<bool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on using a bit in the ref_count
here over a full bool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hesitant to keep the mark bit in ref_count
since there was 1~2% overhead, but it seems like putting the mark bit into non_root_count
does not have that much of overhead. Updated to include the mark bit in non_root_count
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work! We can resolve the lints on a follow-up PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @tunz !
Attempt to address the issue #2773.
The existing implementation had an expensive overhead of managing root counts, especially for mutable borrow of GcRefCell. Instead of managing the root counts, this change counts the number of Gc/WeakGc handles located in Gc heap objects and total number of them. Then, we can find whether there is a root by comparing those numbers.
For example, in the following case, there are 2
Gc
s forGc<i32>
type. One is allocated on the stack, and the other one is located in another Gc heap object.Then, we'll iterate and trace the two objects when running GC, and mark the referenced
Gc
s as non root. We know that there are 2Gc
, but only one of them is marked as non-root. So, we can find that the remaining one is the root.As a trade-off, this approach has an overhead in the the garbage collection time, but I believe there is a room for improvement such as tracing non roots in parallel.
Here is a result of benchmarks:
There is one regression in
Splay
benchmark because it has a lot of DeclarativeEnvironment allocation triggering GC, so it's more impacted by the overhead of GC than benefit of faster property accesses.