Closed Bug 1290550 Opened 4 years ago Closed 4 years ago

Disentangle the mess that is GCRuntime::markRuntime

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- affected
firefox51 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

I'm trying to write an analysis that needs to trace only black roots. Doing this with our current setup would involve passing yet another flags, or worse, just checking for some flag we set randomly elsewhere.

In theory, having all the markers in once place with flags to select what we want should make it clear what is marked when. Unfortunately the flags are not entirely orthogonal in meaning or scope, so this actually only ends up confusing things substantially. Instead of passing flags, I'd like to make 3 orthogonal functions that dispatch to a suite of common functions to do the work:

* traceRuntime: Called by JS::TraceRuntime; sees everything but doesn't care about zones and peers through some weak stuff as well.
* traceRuntimeForMajorGC: Similar to traceRuntime, but is aware of zones, compartments, and gray marking.
* traceRuntimeForMinorGC: Similar, but needs to mark some stuff strongly and simply ignores other things.
Attachment #8776112 - Flags: review?(jcoppeard)
Note, there is a ton of work to do to make this an actual reality. Right now these new entry points just call the existing markRuntime with the proper flags set. I'm going to file incremental fixes blocking this bug.
Depends on: 1290551
Blocks: 1290551
No longer depends on: 1290551
Comment on attachment 8776112 [details] [diff] [review]
1_fix_markruntime_interface-v0.diff

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

Good idea, it would be great to see this cleaned up.

::: js/src/gc/RootMarking.cpp
@@ +350,3 @@
>      rt->spsProfiler.trace(trc);
>  
> +    // Trace the embeddings black and gray roots.

Typo: embedding's
Attachment #8776112 - Flags: review?(jcoppeard) → review+
https://hg.mozilla.org/mozilla-central/rev/c330ce7c763c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.