Clean up cycle collection code

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: peterv, Assigned: peterv)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

34.33 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
12.62 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
59.45 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
2.86 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
2.23 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
We should merge BeginCollection and FinishCollection into a Collect function, since nothing needs to call them separately anymore. Also, currently the participants have three hooks: RootAndUnlinkJSObjects, Unlink and Unroot. Since we don't intertwine GC and CC anymore we should make that Root, Unlink and Unroot and just unlink the JS objects from Unlink too.
(Assignee)

Comment 1

7 years ago
BeginCollection and FinishCollection need to be separate again since we moved stuff to a different thread, but there is still a bunch of cleanup we can do.
Summary: Merge nsCycleCollector::BeginCollection and ::FinishCollection → Clean up cycle collection code
(Assignee)

Comment 2

7 years ago
Created attachment 517157 [details] [diff] [review]
Move unlinking of JS members from root into unlink

We needed to do unlinking of JS members from Root because we needed to forget those references before marking black objects and GC sweeping, which happened between Root and Unlink. Since we don't run the GC during the CC anymore we don't need to do that anymore, we can just unlink all members from Unlink.
Attachment #517157 - Flags: review?(bent.mozilla)
(Assignee)

Comment 3

7 years ago
Created attachment 517158 [details] [diff] [review]
Rename RootAndUnlinkJSObjects to Root

Given that we don't unlink from Root anymore.
Attachment #517158 - Flags: review?(bent.mozilla)
(Assignee)

Comment 4

7 years ago
Created attachment 517159 [details] [diff] [review]
Remove NS_IMPL_CYCLE_COLLECTING_ADDREF_AMBIGUOUS/NS_IMPL_CYCLE_COLLECTING_RELEASE_AMBIGUOUS

We made NS_IMPL_CYCLE_COLLECTING_ADDREF_AMBIGUOUS/NS_IMPL_CYCLE_COLLECTING_RELEASE_AMBIGUOUS identical to NS_IMPL_CYCLE_COLLECTING_ADDREF/NS_IMPL_CYCLE_COLLECTING_RELEASE a long time ago (they don't use the _basetype argument anymore). We might as well remove them.
Attachment #517159 - Flags: review?(bent.mozilla)
(Assignee)

Comment 5

7 years ago
Created attachment 517160 [details] [diff] [review]
Merge RootWhite and CollectWhite

Again made possible because we don't run GC during CC anymore.
Attachment #517160 - Flags: review?(bent.mozilla)
(Assignee)

Comment 6

7 years ago
Created attachment 517161 [details] [diff] [review]
Add some edge names

Not really cleanup but whatever :-).
Attachment #517161 - Flags: review?(bent.mozilla)
Comment on attachment 517157 [details] [diff] [review]
Move unlinking of JS members from root into unlink

This all looks good.
Attachment #517157 - Flags: review?(bent.mozilla) → review+
Attachment #517158 - Flags: review?(bent.mozilla) → review+
Attachment #517159 - Flags: review?(bent.mozilla) → review+
Attachment #517160 - Flags: review?(bent.mozilla) → review+
Comment on attachment 517161 [details] [diff] [review]
Add some edge names

>+                NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "tearoffs mNative");

Nit: "tearoff's mNative" or "tearoff->mNative"
Attachment #517161 - Flags: review?(bent.mozilla) → review+
Depends on: 648720
You need to log in before you can comment on or make changes to this bug.