When doing the off thread parsing stuff I measured how much overhead the use of a lock for accessing the atoms table during parsing as being about 10%. We usually don't eat this overhead currently as the lock is only taken on the main thread if there are any pending or in progress off thread parses. With bug 964059 though we would always need to lock accesses on the atoms zone, as DOM workers could be doing arbitrary stuff requiring use of the table. It would be nice to be able to do bug 964059 without needing to eat this overhead, and it would be possible to do this by minimizing the number of discrete times the atoms table is accessed in the frontend (this is the only significant source of atomization in the VM, so far as I know). I think this could be done by using (thread) locally atomized canonical names in the tokenizer and parser, and converting all of these into atoms in one go before the emitter starts. The atoms table lock would only need to be taken once, around the batch conversion of all the parser's canonical names.

It seems like this would remove the limitation that off-thread parsing cannot run when rt->activeGCInAtomsZone. Is that right? That'd be great!

(In reply to Luke Wagner [:luke] from comment #1) > It seems like this would remove the limitation that off-thread parsing > cannot run when rt->activeGCInAtomsZone. Is that right? That'd be great! Not quite, the main reason for the activeGCInAtomsZone limitation is that we can't trip pre barriers on writes of atoms during off thread parsing, since the barrier tracer isn't thread safe. Even after this patch we will still be writing atoms when e.g. filling in the scripts at the end, and while most of this is initialization (and won't trip pre barriers) I don't think it all is. In any case, doing bug 964059 would remove the need for the activeGCInAtomsZone limitation, as in that case we'd need to be able to handle DOM workers doing arbitrary stuff with the atoms zone while the main thread is in the middle of an IGC.

After trying this approach for a bit it seems doomed. The parser is pretty dependent on other parts of the VM and in certain places stores atoms in heap structures, or computes hash keys based on atom addresses. Fixing this seems a boondoggle and not worth the effort. As an alternative approach, the attached patch amortizes the cost of repeatedly taking the exclusive access lock during parsing by making it sticky --- during parsing, AutoLockForExclusiveAccess only occasionally releases the lock, so that the next time it does not need to reacquire it. This patch also removes the numExclusiveThreads bypass for the lock, and e.g. doesn't affect octane-codeload scores for me.

Oh, the patch rev is e3d23c3d9dc5. Going to hold off on trying to land this until the approach for bug 964059 comes into clearer focus.