One potential issue here is that an assertion failure in Nightly
builds might be simultaneously very annoying (because it crashes the
browser of Nightly users every time it occurs) and not very actionable
(because no reliable STR can be found for it). That's the situation
I'm in with bug 1457603, for example, where I may have to end up
_downgrading_ a diagnostic assert that I added, to a regular assert.
On Mon, Sep 17, 2018 at 3:25 PM, Jeff Gilbert <jgilb...@mozilla.com> wrote:
> I would be excited for something between MOZ_ASSERT and MOZ_CRASH, but
> I think raising MOZ_ASSERTs to MOZ_ASSERT_NIGHTLY should be done by
> hand and reviewed.
>
> On Mon, Sep 17, 2018 at 11:46 AM, Kris Maglione <kmagli...@mozilla.com> wrote:
>> There are some non-trivial snags for this proposal. A lot of assertions rely
>> on other code gated on #ifdef DEBUG or DebugOnly<...> to avoid defining data
>> members or storing values in non-debug builds. We could replace those with
>> more fine-grained macros, of course, but particularly in the case of data
>> members, we'd probably also take a significant memory usage regression.
>>
>> There's also the snag of NS_ASSERTION macros which are... weird.
>>
>> This is nothing we can't deal with, of course, but it will probably require
>> a lot of manual work if we want to do it correctly.
>>
>>
>> On Mon, Sep 17, 2018 at 02:38:14PM -0400, Randell Jesup wrote:
>>>
>>> There are quite a few things that may be caught by assertions by
>>> developers before committing, especially sec issues - but most
>>> developers don't run debug builds as their main local test builds, or
>>> for local browsing use, because they are almost unusably slow. And
>>> turning on compiler optimization doesn't actually help much here - the
>>> problem is mostly debug-only assertions and code that's debug only, such
>>> as bug 1441052 ("Don't do full grey-node checking in local debug
>>> builds").
>>>
>>> These added checks may be useful for CI tests, but they make the builds
>>> unusable, so the vast majority of assertions don't run in the builds our
>>> developers are using. So enabling most of the existing MOZ_ASSERTs for
>>> local opt builds (minus the worst performance-limiting ones) would help
>>> developers catch bugs before landing them (and perhaps shipping
>>> them). It's a lot like using ASAN builds to browse - but with much less
>>> perf degradation on hopes.
>>>
>>> Even better, if we can make the overall hit to perf low enough (1%?), we
>>> could enable it for some/all Nightly users/builds. Or make "early
>>> nightly" (and developer builds) enable them, and late nightly and beta
>>> not.
>>>
>>> If we moved some of the most expensive checks to an alternative macro
>>> (MOZ_ASSERT_DEBUG()), we could (selectively?) enable MOZ_ASSERT checks
>>> in some opt builds. Alternatively we could promote some MOZ_ASSERTs to
>>> MOZ_ASSERT_NIGHTLY() (or MOZ_DIAGNOSTIC_ASSERT), which would assert in
>>> (all) debug builds, and in nightly opt builds - and maybe promote some
>>> to MOZ_ASSERT_RELEASE if we can take the perf hit, and they're in an
>>> important spot.
>>>
>>> And as a stepping-stone to the above, having local opt builds enable
>>> (most) MOZ_ASSERTs (excluding really expensive ones, like bug 1441052)
>>> even if the hit is larger (5, 10%) would also increase the likelihood
>>> that we'll catch things before we land them, or before they get to beta.
>>> (Note: --enable-release would turn this local-build behavior off - and
>>> anyone doing speed tests/profiling/etc needs that already, and probably
>>> we should have a specific enable/disable like
>>> --disable-extra-assertions). This wouldn't be all that hard - most of
>>> the work would be finding "expensive" assertions like bug 1441052.
>>>
>>> (and perhaps we should not continue to overload --enable-release for "I
>>> want to benchmark/profile this build")
>>>
>>> Enabling most MOZ_ASSERTs in automation tests on opt builds would also
>>> slightly increase our odds of finding problems - though this may not
>>> help much, as the same tests are being run in debug builds. The only
>>> advantage would be races may be more likely in the faster opt builds.
>>> It might be worth trying once we have this for a few weeks, and see if
>>> it helps or not.
>>>
>>> Note: I've discussed this concept with various people including bz in
>>> the past, and mostly have had agreement that this would be useful - thus
>>> my filing of bug 1441052. If we agree this is worth doing, we should
>>> file bugs on it and see what the effort to get this would be, and if we
>>> could ship some of this to users. Much like nightly-asan, this would
>>> shake out bugs we'll never find from crash-stats reports, and which can
>>> lead to critical sec bugs, and turn them into frequently-actionable
>>> bugs. If needed this can be enabled incrementally once the build/config
>>> infra and macros are in place; there are several options for that.
>>
>> _______________________________________________
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
