1285305 - Document the UpdatePhase stuff

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

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

shu was asking about whether his new GC thing should go into UpdatePhaseMisc, which was the first I'd seen this UpdatePhase stuff. It needs better documentation, because right now it seems to imply that nothing in UpdatePhaseMisc points to objects, which is false. (I think most of them do.) I don't understand exactly what's going on, but the questions I would ask are:

 - "...because of dependencies between the different types of GC thing." What sort of dependencies, since it doesn't seem to be a simple "can point to" dependency. And why?

 - it says 3 phases, but it's really 4. TypeDescrs come before objects, though they're not distinguishable by AllocKind, I guess. So it's really about partitioning the various GC things, and some of the partitions happen to neatly follow AllocKind boundaries. (And I guess TypedDescrObjects get re-scanned during the objects phase, it's just that nothing will happen to them then? Might be worth a comment.)

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1285305-document-compacting-phases

Comment 2

[Jon Coppeard (:jonco)]

Actually I can't see why we need a separate phase for base shapes any more.  I added that to fix bug 1284388 (but without explaining why).  It may be that it is not necessary any more following other changes.

Comment 3

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

Comment on attachment 8773254 [details] [diff] [review]
bug1285305-document-compacting-phases

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

Now it makes sense, thank you!

::: js/src/jsgc.cpp
@@ +2473,5 @@
> +//
> +// The main dependencies are:
> +//
> +//   - Updating a shape makes use of its base shape
> +//   - Updated a JSObject makes use of its shape

*Updating

@@ +2476,5 @@
> +//   - Updating a shape makes use of its base shape
> +//   - Updated a JSObject makes use of its shape
> +//   - Updating a typed object makes use of its type descriptor object
> +//
> +// This means we requrie at least four phases for update:

*require

@@ +2483,5 @@
> +//  2) shapes
> +//  3) typed object type descriptor objects
> +//  4) all other objects
> +//
> +// Since we want to number minimize the number of phases, we put everything else

s/number //

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32050e8124af
Add comments explaining compacting GC update phases r=sfink

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/32050e8124af