There's a whole bunch if isMarkedFromAnyThread methods here that don't seem to be called from anywhere.
Created attachment 8853021 [details] [diff] [review] bug1352103-jitcode-map-marking Remove unused code.
Yeah, I have a patch for that too. Thought I'm having second thoughts about it. What appears to be going on is that the marking depends on some complex and fragile-sounding assumptions, and that dead code was meant for testing whether the assumptions held. But it looks like the test must have broken. I haven't looked at blame, but I'm guessing that's what this comment is referring to: // JitcodeGlobalEntries are marked at the end of the mark phase. A read // barrier is not needed. Any JS frames sampled during the sweep phase of // the GC must be on stack, and on-stack frames must already be marked at // the beginning of the sweep phase. It's not possible to assert this here // as we may not be off thread when called from the gecko profiler. Or in more detail: // JitcodeGlobalTable must keep entries that are in the sampler buffer // alive. This conditionality is akin to holding the entries weakly. // // If this table were marked at the beginning of the mark phase, then // sampling would require a read barrier for sampling in between // incremental GC slices. However, invoking read barriers from the sampler // is wildly unsafe. The sampler may run at any time, including during GC // itself. // // Instead, JitcodeGlobalTable is marked at the beginning of the sweep // phase, along with weak references. The key assumption is the // following. At the beginning of the sweep phase, any JS frames that the // sampler may put in its buffer that are not already there at the // beginning of the mark phase must have already been marked, as either 1) // the frame was on-stack at the beginning of the sweep phase, or 2) the // frame was pushed between incremental sweep slices. Frames of case 1) // are already marked. Frames of case 2) must have been reachable to have // been newly pushed, and thus are already marked. // // The approach above obviates the need for read barriers. The assumption // above is checked in JitcodeGlobalTable::lookupForSampler. That last part is now a lie. I'm guessing that was originally what the dead code was doing. And yet, it seems like it would be a really good thing to test that assumption. Is there a way to do that?
Comment on attachment 8853021 [details] [diff] [review] bug1352103-jitcode-map-marking Cancelling review until we work out whether we want to keep this code around.