811042 - BaselineCompiler: Mark shared stubcodes so they don't get GCed.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Kannan Vijayan [:djvj]]

We need to gc-protect the shared stubcodes.  For now, just mark all shared stubcodes as rooted at the IonCompartment.  Later on, we may want to mark them from the BaselineScripts they are used by, and reclaim unused ones.

For now this should do.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: Patch.

Hooks in shared stub code map with the IonCompartment's mark and sweep mechanisms.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

We have to be careful about rooting in the IonCompartment - everything "rooted" by a JSCompartment must be a weak root, since JSCompartments are only destroyed when they have no live objects.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 680735 [details] [diff] [review]
Patch.

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

Patch LGTM, but it would be good to avoid the issue in comment 2 before we forget about it. Can we store the IonCode pointer in ICStub for now and mark that in BaselineScript::trace? Once we can run more code we can measure the number of stubs in the browser and see if we need something more clever. (Later on we could also benchmark not storing the raw pointer but loading it from the IonCode).

Comment 4

[Kannan Vijayan [:djvj]]

Comment on attachment 680735 [details] [diff] [review]
Patch.

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

Ok, these are good points.  Canceling review until we address this properly down the line.

Comment 5

[Kannan Vijayan [:djvj]]

Attachment: Mark StubCodes

This patch moves the stubCodes_ map into IonCompartment instead of IonRuntime.  It adds marking of used stubcodes from the BaselineScripts where they are used, and sweeping of the cache map when the IonCompartment is swept.

This patch assumes an unmerged mozilla-inbound patch which adds a fallible |initialize| method to IonCompartment construction: https://hg.mozilla.org/integration/mozilla-inbound/rev/bd4d746598fd

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 683684 [details] [diff] [review]
Mark StubCodes

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

Looks good, I'm glad we don't have to store a separate IonCode pointer. (As discussed on IRC, don't forget to add the sweep call in IonCompartment::sweep).

Comment 7

[Kannan Vijayan [:djvj]]

https://hg.mozilla.org/projects/ionmonkey/rev/abe6255ed9e7