Compartment per global introduced some compartment mismatches, and there may be more that we haven't caught yet. The GC crash rate for FF15 seems to be higher than it is for FF14. Hopefully we can do these checks for a while on Nightly, fix any bugs that arise, and port the fixes to beta.

(In reply to Bill McCloskey (:billm) from comment #0) > Hopefully we can do these checks for a while on > Nightly, fix any bugs that arise, and port the fixes to beta. Sounds good, but keep in mind that the beta ship is about to sail in an hour or so.

This is green on tryserver. There were some fairly minor benchmark regressions in the shell because we do compartment checks on all calls to natives. That makes, for example, v8-regexp a lot slower. I think these native checks are important, though. I just did another push to compare Talos numbers.

Well, there are some regressions, but that's to be expected. Here's a comparison: without asserts: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=9f49d90a18c7 with asserts: https://tbpl.mozilla.org/?tree=Try&rev=be35eebfd0df I don't see anything bigger than ~10%. That seems reasonable to me for a patch that will only land on the nightly channel.

It seems like a good idea to leave the cheaper checks on all the time, not just temporarily to catch bugs. This suggests a slight name change, say s/assert/check/. To make it even easier going forward, is there a precompiler symbol we can use distinguish nightly from other release channels. This would give us: checkSameCompartment // always run in release, all JSAPI entry points checkSameCompartmentOnNightly // check in release, but only on nightly, otherwise in debug assertSameCompartment // only debug

We have JS_CRASH_DIAGNOSTICS, which should only be enabled on nightly. We could make the native calls assert only if that's defined, and otherwise keep the split in the patch. I'll rerun all the benchmarks tomorrow without the native call asserts and see how much that costs us. However, I'm worried that non-JS people will not be so accepting of taking a permanent 10% hit on Talos, even if it's only on Nightlies.

Yeah, you're right; any perf hit we take on Nightly should be temporary.

Maybe later, when we turn assertSameCompartment back to debug-only, we could add checkSameCompartment and use it for all JSAPI entry points.

I pushed this to tryserver and the Talos results actually look fine. There might be a few small regressions, but nothing major. I conditioned everything on JS_CRASH_DIAGNOSTICS in case we decide to leave this in. That way the assertions will be restricted to nightly. https://hg.mozilla.org/integration/mozilla-inbound/rev/a94cbe677ea7