Closed Bug 820422 Opened 8 years ago Closed 8 years ago

GC: Store buffered gray roots per-compartment

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(1 file, 1 obsolete file)

At the moment all the buffered gray roots are stored in the runtime.  This means that we have to iterate through all of them when marking a particular compartment.

A quick look at the number of roots showed that most compartments have few (if any) and some have very many, so I think it makes sense to split them up and store them by compartment.
Attached patch Proposed fix (obsolete) — Splinter Review
Assignee: general → jcoppeard
Attached patch Proposed fixSplinter Review
Updated patch, fixed a couple of minor issues.
Attachment #690912 - Attachment is obsolete: true
Attachment #691307 - Flags: review?(wmccloskey)
Comment on attachment 691307 [details] [diff] [review]
Proposed fix

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

::: js/src/gc/RootMarking.cpp
@@ +798,5 @@
>      }
>  }
> +
> +void
> +js::gc::BufferGrayRoots(JSTracer *trc)

Can this take a GCMarker* instead of a JSTracer*?

::: js/src/jsgc.cpp
@@ +3556,5 @@
>      JS_ASSERT_IF(lastGC, !rt->gcSweepOnBackgroundThread);
>  
>      JS_ASSERT(rt->gcMarker.isDrained());
>      rt->gcMarker.stop();
> +    rt->gcMarker.resetBufferedGrayRoots();

Could GCMarker::stop call resetBufferedGrayRoots? Then this call and the one below wouldn't be needed.
Attachment #691307 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/37b4c4b9d7d6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Depends on: 821762
You need to log in before you can comment on or make changes to this bug.