GC: Simplify compartment GC state

RESOLVED FIXED in mozilla17

Status

()

Core
JavaScript Engine
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
In preparation incremental compartment sweeping, separate out the 'scheduled for GC' flag from 'incremental GC in progress' flag for compartments.
(Assignee)

Comment 1

5 years ago
Created attachment 651441 [details] [diff] [review]
Proposed fix
Attachment #651441 - Flags: review?(wmccloskey)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac17d5c3d370
https://hg.mozilla.org/mozilla-central/rev/ac17d5c3d370
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.