With webrender we've had pretty good results from always defaulting to
MOZ_RELEASE_ASSERT, as users are often mildly decent at producing an STR.
But perhaps we're luckier in that everyone dogfooding webrender was more
willing to figure things out than a nightly user?
On Mon, Sep 17, 2018 at 3:31 PM Botond Ballo <bba...@mozilla.com> wrote:
> 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
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
