1341044 - Rename the GC's concept of a zone group now we also have ZoneGroup

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

Before there was ZoneGroup, the GC had a concept of a group of zones that were swept at the same time as part of incremental sweeping.  We should rename these to something else to avoid confusion.

Comment 1

[Steve Fink [:sfink] [:s:]]

Hrm, naming.

zone batch
zone sweep group
zone connected set
zone connected component
zone component
zone clique (but I think that's a misuse here)
zone hairball
zone closure (from "transitive closure")
sweep group
zone collection
megazone
zoneball
superzone

Comment 2

[Andrew McCreight [:mccr8]]

How about "zone sweep suite".

Comment 3

[Steve Fink [:sfink] [:s:]]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> How about "zone sweep suite".

Only if you post a video proving that you can say it properly 10 times in a row, in less than 10 seconds.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: bug1341044-rename-gc-zone-group

I went with 'sweep group', which is a little clunky in places (e.g. beginSweepingSweepGroup) but at least it's explicit.  

For the public callbacks I chose e.g. JS_AddWeakPointerZonesCallback to go with the existing JS_AddWeakPointerCompartmentCallback.

Comment 5

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8849953 [details] [diff] [review]
bug1341044-rename-gc-zone-group

Review of attachment 8849953 [details] [diff] [review]:
-----------------------------------------------------------------

WFM. And some of the new names are *much* better; it really helps to have "sweep" in the name to avoid confusion over why the zones are being grouped.

::: js/src/gc/Zone.cpp
@@ +164,1 @@
>                  // groups. In this case both script and debugger object must be

Missed one at EOL.

::: js/src/jsgc.cpp
@@ +80,4 @@
>   *
>   * Slice n+1: Sweep:      Mark objects in unswept zones that were newly
>   *                        identified as alive (see below). Then sweep more zone
>   *                        groups.

Missed one here. And I'd probably spell it out as "zone sweep groups" here, though you can leave the shorter "zones" in the next one.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b886ec9acd35
Rename the GC's 'zone group' concept to 'sweep group' r=sfink

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b886ec9acd35