817493 - Use templates for compartment group finder

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

Using a virtual function inside JSCompartment is causing problems in bug 747066. This is my fault, and Jon originally wrote the patch using a template. This patch reverts back to using templates and removes the virtual.

I also did some refactoring of the compartment group finder. Now it constructs a single list of all the compartments. The list is ordered so that each group is contiguous. Each compartment has a link to the next compartment and to the first compartment in the next group. This seems easier to understand to me, since the list doesn't get updated after it's first constructed.

Finally, I moved the stack overflow handling to happen at the end of the algorithm rather than in the middle. This seemed easier to reason about.

Comment 1

[Jon Coppeard (:jonco)]

Comment on attachment 687630 [details] [diff] [review]
patch

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

Looks good!

One comment: in the processNode method, in the loop that adds nodes to the list - all nodes added are in the same component, so I think gcNextGraphComponent can always be set to whatever firstComponent was at the start of the loop.  Also, we don't need to update gcLowLink here any more, that was to record the components before you added gcNextGraphComponent.

You've improved the handling of stack overflow too I see.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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

Just noticed I forgot about the comments. I'll fix that tomorrow.

Comment 3

[Ed Morley [:emorley]]

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

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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

Comment 5

[Ed Morley [:emorley]]

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