Closed Bug 639270 Opened 9 years ago Closed 9 years ago

sweep compartments when they are empty, no longer mark them

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: gal, Assigned: gal)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 3 obsolete files)

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.
Depends on: 639254
Assignee: general → gal
Attached patch patch (obsolete) — Splinter Review
Attachment #517231 - Flags: review?
Attachment #517231 - Flags: review? → review?(anygregor)
> 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
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?
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).
Attached patch patch (obsolete) — Splinter Review
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.
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.
Blocks: 639729
Attached patch patch (obsolete) — Splinter Review
Attachment #517231 - Attachment is obsolete: true
Attachment #517288 - Attachment is obsolete: true
Attachment #517231 - Flags: review?(anygregor)
Attached patch patchSplinter Review
Attachment #517884 - Attachment is obsolete: true
Attachment #517885 - Flags: review?(anygregor)
Comment on attachment 517885 [details] [diff] [review]
patch

Some overhead for this corner case but looks good.
Attachment #517885 - Flags: review?(anygregor) → review+
Global objects are very very rarely allocated.
http://hg.mozilla.org/tracemonkey/rev/80d8431f209e
Whiteboard: fixed-in-tracemonkey
    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
OS: Mac OS X → All
Backed out in http://hg.mozilla.org/tracemonkey/rev/38aba506e3c4 - nits or not, it doesn't actually compile.
Whiteboard: fixed-in-tracemonkey
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.
Missing the missing renaming in xpconnect and the nits.

http://hg.mozilla.org/tracemonkey/rev/206d6b6b392f
I'm pretty sure this is fixed, but please unset if I'm wrong.
Whiteboard: fixed-in-tracemonkey
Fixed yes, merged to m-c no, so we are keeping the bug open.
http://hg.mozilla.org/mozilla-central/rev/206d6b6b392f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.