Closed Bug 1285305 Opened 7 years ago Closed 6 years ago

Document the UpdatePhase stuff


(Core :: JavaScript: GC, defect)

Not set



Tracking Status
firefox50 --- fixed


(Reporter: sfink, Assigned: jonco)




(1 file)

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.)
Assignee: nobody → jcoppeard
Attachment #8773254 - Flags: review?(sphink)
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 on attachment 8773254 [details] [diff] [review]

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


@@ +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:


@@ +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 //
Attachment #8773254 - Flags: review?(sphink) → review+
Pushed by
Add comments explaining compacting GC update phases r=sfink
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.