Re: [blink-dev] Proposal: More aggressive RELEASE_ASSERTs

[David Bokan]

+chromium-dev@

 

This seems more appropriate on +chromium-dev@ so bcc:blink-dev@

It seems like there was some support for doing this in those old threads, did anything ever come of it? Do people think this is a problem worth solving?

I took a look at Blink's RELEASE_ASSERT which seems more lightweight than CHECK. On a Linux build, it generates 2 additional instructions (8 bytes) plus the check itself. Turning all of Blink's ASSERTs into RELEASE_ASSERT (but leaving off ASSERTs that add data to objects just for asserting) added 3.8% to Blink's binary size. For a Canary/Dev build I feel that's a good tradeoff.

pkasting@ above mentioned that turning on dchecks slows down the browser a lot. This is true. I used the http://peacekeeper.futuremark.com/ benchmark just to get a sense and turning on dchecks caused the score to drop from 3700 to 1200. Using RELEASE_ASSERT instead of ASSERT in Blink, as above, scored 2400. Clearly we wouldn't want to do this in stable channel. However, I didn't notice a difference until I tried the benchmark, general browsing remains quite usable.

If there's will, I think there's some pragmatic and incremental steps we could take. Adding a new "field checked" assert and using that in place of DCHECKs where it makes sense going forward means at least future asserts would be more accurate. As was floated in the past, compiling this only into canary/dev builds would probably be the best way forward. If we're worried about losing users in those channels, we could use UMA to report on failing ASSERTs in release rather than crashing.

On Tue, Oct 6, 2015 at 2:25 PM, Dana Jansens <dan...@chromium.org> wrote:

On Tue, Oct 6, 2015 at 2:16 AM, 'Peter Kasting' via blink-dev <blin...@chromium.org> wrote:

On Mon, Oct 5, 2015 at 11:04 PM, Yoav Weiss <yo...@yoav.ws> wrote:

Couldn't we release the binaries for Canary/Dev (and maybe Beta) with dcheck_always_on?

That way we would catch more asserts than we currently do without impacting stable.

DCHECK_ALWAYS_ON causes significant slowdowns.  Too significant for Dev or Beta and too significant for more than occasional Canary runs.

Some previous threads on this:

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/c2X0D1Z6X2o/XyS16tj8zdQJ

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/xSPAZEoZPd0/lP41NyJLEPsJ

In https://groups.google.com/a/chromium.org/d/msg/chromium-dev/c2X0D1Z6X2o/nodcjy2lewYJ there was a proposal to run with dchecks enabled sometimes in canary.

[David Bokan]

Sorry, I messed up the thread-chain in moving it to chromium-dev. Here's the link on blink-dev@: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/jC66QpPhyow

[Alexei Svitkine]

FWIW, I still stand by my previous suggestion on an earlier thread (that someone already linked above):

https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/c2X0D1Z6X2o/nodcjy2lewYJ

It would still result in a binary size increase for canary users, but would allow us to get data from the wild without the majority of users to suffer the runtime penalties (perf hit / crash rate increase).

(But someone actually has to drive this - I'm not stepping up to do that, though happy to consult if someone wants to take this on.)

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Jakob Kummerow]

My personal suggestion would be to create an auto-updating build with DCHECK_ALWAYS_ON (which I guess includes Blink ASSERTs?) and encourage all Chromium developers to use that for their own browsing. That way, failing DCHECKs would annoy exactly those people who are in a position to do something about them, without impacting any regular users.

[Ryan Hamilton]

Alternatively, we could build such a version of chrome, and set up a bot which, say, runs Chrome against some popular sites automatically, reporting any DCHECKS that fire. This way we find the problems without forcing chrome devs to run a slow version of chrome. I think we had such a bot back in the day....

[David Bokan]

I like Alexei's old proposal, pasted here for convenience:

If we enable dchecks for 1% or (even 0.1%) of the runs (randomized on startup, so a user can just restart and get put in a different group if they get a bad experience), I don't think it should make canary a noticeably worse experience for most users while still feeding us useful data about issues found in the wild. This can be done with a session-randomized field trial.

I think it strikes the right balance between not affecting user experience too much but still giving us useful feedback from real world usage patterns.

I'd be willing to put some time into experimenting on this in this quarter unless there are any strong objections. I'll put together a more specific and detailed proposal shortly.