1190911 - Remove AutoIdValueVector and use standard rooting

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

Attachment: remove_autoidvaluevector-v0.diff

And for IdValuePairs. I had no idea these even existed.

Since this is used in the front-end, I also tracked down how rooting on the parse thread works. Only actual JSContexts are in the Runtime list that ContextIter visits, so we simply don't mark Rooted that are linked into an ExclusiveContext. This works because we cannot collect the parse thread zone. I do not think, however, that we have assertion anywhere that the thing being rooted actually belongs to the zone (or atoms zone) when off thread, so that is one potential hole.

However, this is a bit spooky-action-at-a-distance. I was thinking that we might want to add a RootLists to Zone in any case so that we can move most of Compartment::trace and Zone::trace to PersistentRooted. This would also let us associate these Rooted directly with the owner Zone, which could dramatically lower our compartment GC overhead.

Comment 1

[Jon Coppeard (:jonco)]

Comment on attachment 8643108 [details] [diff] [review]
remove_autoidvaluevector-v0.diff

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

::: js/src/ds/IdValuePair.h
@@ +22,1 @@
>      jsid id;

Any reason you swapped these around?  The original order makes more sense in light of the name of the class.

Comment 2

[Terrence Cole [:terrence]]

(In reply to Jon Coppeard (:jonco) from comment #1)
> Comment on attachment 8643108 [details] [diff] [review]
> remove_autoidvaluevector-v0.diff
> 
> Review of attachment 8643108 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/ds/IdValuePair.h
> @@ +22,1 @@
> >      jsid id;
> 
> Any reason you swapped these around?  The original order makes more sense in
> light of the name of the class.

On 32bit it should give more efficient packing. I guess it won't matter in a vector because of alignment, but it just seemed like good hygiene.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe4520e7fc27

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Terrence Cole [:terrence] from comment #2)
Oh right, makes sense.