1466171 - Remove the restriction about not collecting atoms while the main thread is parsing

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement)

Description

[Jon Coppeard (:jonco)]

Currently we have this restriction that you can't collect the atoms zone while the main thread is parsing because we don't have exact rooting information about the atoms in use by the parser.  We use AutoKeepAtoms to signal the situation.

I'd like to remove this restriction.  The zone keeps a cache of all the atoms that have been looked up since the last GC, and that should include all the atoms that the parser is using (we will have to make sure we don't clear it while parsing).

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1466171-keep-atoms

Move the keepAtoms counter to the zone and remove the restriction on collecting atoms while AutoKeepAtoms is present.  Add a flag to prevent clearing the zone's atoms cache while we're parsing and clear it at the end of GC if needed.

Try is green so far: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bebada7c0f2198d7f4f8fd09dbf8ebf46c4a22ed

Comment 2

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

Comment on attachment 8982579 [details] [diff] [review]
bug1466171-keep-atoms

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

Nicely done. The scary part is what else we might have been covering up under the AutoKeepAtoms safety blanket.

::: js/src/gc/RootMarking.cpp
@@ +334,5 @@
>  void
> +js::gc::GCRuntime::traceKeptAtoms(JSTracer* trc)
> +{
> +    // We don't have exact rooting information for atoms while parsing. When
> +    // this is happeninng we set a flag on the zone and trace all atoms in the

*happening

::: js/src/gc/Zone.h
@@ +509,5 @@
> +    // collecting the atoms zone when exclusive threads are running.
> +    js::ZoneOrGCTaskData<unsigned> keepAtomsCount;
> +
> +    // Whether purging atoms was deferred due to keepAtoms being set. If this
> +    // happen then the cache will be purged when keepAtoms drops to zero.

*happens

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d741e95201a
Allow collection of atoms while the main thread is parsing r=sfink

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6d741e95201a