Closed Bug 817493 Opened 10 years ago Closed 10 years ago

Use templates for compartment group finder


(Core :: JavaScript Engine, defect)

Not set





(Reporter: billm, Assigned: billm)



(1 file)

Attached patch patchSplinter Review
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.
Attachment #687630 - Flags: review?(jcoppeard)
Comment on attachment 687630 [details] [diff] [review]

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.
Attachment #687630 - Flags: review?(jcoppeard) → review+

Just noticed I forgot about the comments. I'll fix that tomorrow.
Flags: needinfo?(wmccloskey)
Whiteboard: [leave open]
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.