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
