After bug 1592981 our compiler is a little more choosy about what functions it will inline during LTO. The presence of string canaries can now knock some key functions out-of-line, at a cost of more than 20% regressions on some talos subtests.

I was frustrated to spend a lot of time digging into the regression only to find that it wouldn't have mattered on our release builds anyway. It doesn't seem great that nightlies, where we do our perf work, are so different from the builds that users will get.

I understand the temptation to leave the canaries in because they could find good bugs -- or even have found good bugs -- but I don't see any investigations in bug 1410276 and friends for 10 months. Could I ask that we remove the canaries for now, and if needed, reland them only for short periods when we can commit to actively looking into the results?

I know I put the canaries in, I remember doing it but I don't really remember why. I'm happy to remove them at this point, they seem to have done their job (AFAIK). But I'm also happy for someone who is more familiar with this part of code to weigh in.

According to Bug 1405525 and Bug 1410276 it looks like I own the responsability for these cannary values. So I guess I should just make a decision (I'll remove them).

Alternatively we can just disable this code for nightly, but keep it enabled for debug. That'll fix the inlining problem for dmajor but it keeps the canary checking in there. But that's probably not useful because we already run asan as much as debug and asan may be protecting against 99% of the cases the canary can protect for. Just a thought in case anyone wanted to keep this for debug builds.

bz do you have any thoughts since I added this canary based on Bug 1403397 comment 26

If we're not making use of the canary bits, we should indeed remove them...

https://hg.mozilla.org/mozilla-central/rev/152f204eda63

Thanks Paul!

(In reply to :dmajor from comment #7)

Thanks Paul!

Np.