1252556 - Failure to profile application under profileserver.py went undetected

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

As part of working on VS 2015 support, I produced a build that crashed on startup.

To my surprise, profileserver.py nor any other part of the build complained that firefox.exe crashed on startup! No .pgc files were produced and the 2nd pass of the linking continued as if everything were OK. link.exe did complain about missing .pgc files, however. But it wasn't a fatal error.

As a double whammy, the 2nd link.exe took longer to complete than a typical PGO 2nd link! I reckon the reason is it was performing whole program optimization over all of the code as opposed to targeting the parts the PGO profile data says to target.

I think the build system should detect incomplete PGO profiling runs and fail fast.

Comment 1

[Gregory Szorc [:gps]]

This doesn't block VS2015 landing.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1252556 - Make profileserver.py fail when running Firefox fails.

Currently, when Firefox crashes when running the PGO profile, the error
is ignored, and the result is a build that is not optimized as expected.
So instead of such things going unnoticed, we make profileserver.py
return an error code, further failing the build.

Unfortunately, this does not make the output useful to know what went
wrong exactly, but the most important thing is that the build fails
instead of going through and leading to bad results.

Review commit: https://reviewboard.mozilla.org/r/195614/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195614/

Comment 3

[Mike Hommey [:glandium]]

Note the patch can't land until bug 1413570 is fixed.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8924362 [details]
Bug 1252556 - Make profileserver.py fail when running Firefox fails.

https://reviewboard.mozilla.org/r/195614/#review200782

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8924362 [details]
Bug 1252556 - Make profileserver.py fail when running Firefox fails.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195614/diff/1-2/

Comment 6

[(not currently active) Ted Mielczarek]

As a followup, we should figure out how to get useful stacks out if Firefox crashes during the profiling run. We should get a minidump, the problem has historically been that we don't have symbols. We could probably just forcibly run `make syms-recurse` from this script and then use mozcrash to look for minidumps, since the build is going to fail at that point anyway.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8924362 [details]
Bug 1252556 - Make profileserver.py fail when running Firefox fails.

https://reviewboard.mozilla.org/r/195614/#review201130

Comment 8

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> As a followup, we should figure out how to get useful stacks out if Firefox
> crashes during the profiling run. We should get a minidump, the problem has
> historically been that we don't have symbols. We could probably just
> forcibly run `make syms-recurse` from this script and then use mozcrash to
> look for minidumps, since the build is going to fail at that point anyway.

Well, the patch as attached reveals that linux builds are intermittently crashing at shutdown during the profiling, at a relatively high rate. So the patch can't land as long as that's not fixed, and I spent a lot of time yesterday trying to get something out of loaners, but that was a long exercise of frustration (see bug 1413823 for some of it), culminated by 2FA fun (bug 1413855). I also tried on EC2, using the same docker image, and couldn't reproduce there...

As long as loaners can't allow us to get a core, that followup would be the only way to figure out where exactly we're crashing on shutdown. But I spent already too much energy on this. I'm not going to spend more on this in the short term, sorry.

Comment 9

[Mike Hommey [:glandium]]

This was raised indirectly in http://hubicka.blogspot.com/2018/12/firefox-64-built-with-gcc-and-clang.html:

> Issue I run into this time is bit subtle. It is not visible during testing but it can be seen as following message in the build log:
>
>     MOZ_CRASH(Shutdown too long, probably frozen, causing a crash.) at /aux/hubicka/firefox-2018/release/toolkit/components/terminator/nsTerminator.cpp:219

which may be at least one part of the problem. Unfortunately, the situation with loaners is still not satisfying to debug things. That said, disabling the shutdown hang crash on profile-generate builds could go a long way.

Comment 10

[Mike Hommey [:glandium]]

So here's something funny... I've done 40 PGO builds on try with the patch from this bug attached, and only one failed... on the X11 server not being available ; so, on something unrelated to the shutdown hang thing.

Something obviously changed since last year, since the failure rate was much higher back then. I wonder if it's not "just" that I/O got better on automation in between.

It feels like we should land the existing patch, and deal with any problem that arises in followups.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d5d46cf428
Make profileserver.py fail when running Firefox fails. r=chmanchester

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/e4d5d46cf428

Comment 13

[(not currently active) Ted Mielczarek]

FWIW, there's a pref we can set to bump the shutdown timeout:
https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/modules/libpref/init/all.js#1096