639270 - sweep compartments when they are empty, no longer mark them

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

After landing bug 639254 compartments are completely passive (sweep-only), so we no longer have to mark them. This removes another branch from the mark path.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Brendan Eich [:brendan]]

> After landing bug 639254 compartments are completely passive (sweep-only), ...

I.e., compartments are weak, or compartments contribute no strong refs (root set elements), right?

/be

Comment 3

[Gregor Wagner [:gwagner]]

We added the mark flag because a GC can happen during allocation of the first object. How do you prevent in that case that the compartment gets deleted?

Comment 4

[Andreas Gal :gal]

Right. That was a hack though. We could do a pidgeon hole or a rooter. I will add something (feel free to steal any of the patches if you like).

Comment 5

[Gregor Wagner [:gwagner]]

Attachment: patch

How about changing the active flag to a state flag and let it store the current state of the compartment? We might find better names but currently I use constructing, up and active.

Comment 6

[Andreas Gal :gal]

How about a rooter? We have infrastructure for that. Its only needed in NewGlobalWithCompartment. But yeah this would work too. The auto helper can set it.

Comment 7

[Andreas Gal :gal]

Attachment: patch

Comment 8

[Andreas Gal :gal]

Attachment: patch

Comment 9

[Gregor Wagner [:gwagner]]

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

Some overhead for this corner case but looks good.

Comment 10

[Andreas Gal :gal]

Global objects are very very rarely allocated.

Comment 11

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/80d8431f209e

Comment 12

[Brendan Eich [:brendan]]

    1.54 +        *(holdp = &compartment->hold) = true;

Nesting assignment gratuitously! Why not init holdp in the ctor head?

Also don't auto-only helpers need stub copy ctors and dtors?

/be

Comment 13

[Phil Ringnalda (:philor)]

Backed out in http://hg.mozilla.org/tracemonkey/rev/38aba506e3c4 - nits or not, it doesn't actually compile.

Comment 14

[Andreas Gal :gal]

Compiled here! I will fix, including the nit. Thanks Phil. I hope you make it to one of our summits one day so I can buy you one of those many beers I owe you by now.

Comment 15

[Andreas Gal :gal]

Missing the missing renaming in xpconnect and the nits.

http://hg.mozilla.org/tracemonkey/rev/206d6b6b392f

Comment 16

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

I'm pretty sure this is fixed, but please unset if I'm wrong.

Comment 17

[Andreas Gal :gal]

Fixed yes, merged to m-c no, so we are keeping the bug open.

Comment 18

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/206d6b6b392f