Calls like Maybe<>::ref() current MOZ_ASSERT(isSome()) to make sure you're not referencing a non-existent object. We should probably convert these to release or diagnostic asserts.

We out-of-line the relevant functions because release assertions can generate quite a bit of code, and we'd rather let the compiler determine if these functions should be inlined now. I am open to making these MOZ_DIAGNOSTIC_ASSERTs if we think that would be less risky.

Comment on attachment 8926909 [details] [diff] [review] turn Maybe assertions into release assertions Review of attachment 8926909 [details] [diff] [review]: ----------------------------------------------------------------- Thank you!

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/15b89e515c94 turn Maybe assertions into release assertions; r=bkelly

https://hg.mozilla.org/integration/mozilla-inbound/rev/adfd4d3245467c54507316b9be82b86424585031 Backout 15b89e515c94 (bug 1410186) for causing too many crashes on Nightly.

I backed this out because it was working too well -- bug 1416344 was causing extremely high crash rates in Nightly. bkelly, I believe froydnj will be on leave for some time. Would you like to take over this and ensure it gets relanded once bug 1416344 is fixed?

I don't have time to do much here. I personally think we should fix bug 1410186 and not back this out. Referencing uninitialized memory is not safe and hiding the crashes is not doing our users any favors.

I re-read that other bug and see the crash rates are higher than I would have thought last time I read it. We should definitely get that fixed before re-landing this.

This missed the cut-off for Fx58 before the final merge to Beta, so I grafted the backout there as well. https://hg.mozilla.org/releases/mozilla-beta/rev/b188777d8f972c4eb09aeb9471155751c29f62d6

Merge of backout: https://hg.mozilla.org/mozilla-central/rev/adfd4d324546

It turns out that several bits of a all-platforms try build fail with the previous patch, because OptimizationInfo::CompilerWarmupThreshold doesn't actually get assigned storage, and we need it to have storage for Maybe::valueOr (which takes CompilerWarmupThreshold by reference). This patch moves CompilerWarmupThreshold out of the class definition, which seems to solve the issue. I'll fold this into the previous patch before commit.

Nathan, can we re-land these Maybe release assertions now that the trimmingPolicy bug 1416344 has been fixed?

(In reply to Chris Peterson [:cpeterson] from comment #12) > Nathan, can we re-land these Maybe release assertions now that the > trimmingPolicy bug 1416344 has been fixed? Sure, we could do that. Leaving ni? open to remind myself to put the patches back into landable shape.

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c82274a23aea turn Maybe assertions into diagnostic assertions; r=bkelly

(In reply to Chris Peterson [:cpeterson] from comment #12) > Nathan, can we re-land these Maybe release assertions now that the > trimmingPolicy bug 1416344 has been fixed? Relanded, but as MOZ_DIAGNOSTIC_ASSERT, so as to try to limit the performance impact on release.

Backed out on request from igoldan. From IRC igoldan> froydnj: bug 1410186 caused an increase in installer size

Backout link: https://hg.mozilla.org/mozilla-central/rev/77c06979d9e88979ec96263eccdbd750cb9221a4

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/aab1afd86d77 turn Maybe assertions into diagnostic assertions; r=bkelly

(In reply to Andreea Pavel [:apavel] from comment #17) > Backed out on request from igoldan. From IRC > igoldan> froydnj: bug 1410186 caused an increase in installer size Yup, relanded since that bug is wontfix.

(In reply to Nathan Froyd [:froydnj] from comment #20) > (In reply to Andreea Pavel [:apavel] from comment #17) > > Backed out on request from igoldan. From IRC > > igoldan> froydnj: bug 1410186 caused an increase in installer size > > Yup, relanded since that bug is wontfix. That bug is this bug, I'm assuming you meant bug 1461934. This regressed libxul by 256k [1], but as noted it's nightly/beta only so unlikely to be fixed. [1] https://treeherder.mozilla.org/perf.html#/graphs?timerange=7776000&series=mozilla-inbound,1299711,1,2&zoom=1526348067705.438,1526629766980.3625,126492537.31343284,133059701.49253732&selected=mozilla-inbound,1299711,337469,469582724,2