Closed Bug 782318 Opened 9 years ago Closed 9 years ago

GC: Simplify compartment GC state

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

In preparation incremental compartment sweeping, separate out the 'scheduled for GC' flag from 'incremental GC in progress' flag for compartments.
Attached patch Proposed fixSplinter Review
Attachment #651441 - Flags: review?(wmccloskey)
Assignee: general → jcoppeard
Comment on attachment 651441 [details] [diff] [review]
Proposed fix

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

::: js/src/jscompartment.h
@@ +164,5 @@
>  
>    private:
>      enum CompartmentGCState {
> +        NO_GC,
> +        COLLECTING

We're trying to transition to lowercase names for stuff like this. Can you use NoGC and Collecting?

::: js/src/jsgc.cpp
@@ +3190,5 @@
> +     */
> +    DebugOnly<bool> any = false;
> +    for (CompartmentsIter c(rt); !c.done(); c.next()) {
> +        JS_ASSERT(!c->isCollecting());
> +        for (unsigned i = 0 ; i < FINALIZE_LIMIT ; ++i)

No spaces before the semicolons.

@@ +3198,5 @@
> +    }
> +    JS_ASSERT(any);
> +
> +    /*
> +     * Set up which compartments will be collected.

Can just be one line: /* Set up which compartments will be collected. */

@@ +3201,5 @@
> +    /*
> +     * Set up which compartments will be collected.
> +     */
> +    rt->gcIsFull = true;
> +    for (CompartmentsIter c(rt); !c.done(); c.next()) {

Can this loop be combined with the previous one?

@@ +3670,5 @@
>      for (CompartmentsIter c(rt); !c.done(); c.next()) {
>          c->setGCLastBytes(c->gcBytes, c->gcMallocAndFreeBytes, gckind);
>          if (c->wasGCStarted())
> +            c->setCollecting(false);
> +

Extra space here.
Attachment #651441 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/ac17d5c3d370
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.