[gtk3] Running Firefox during builds fails

RESOLVED FIXED in Firefox 42

Status

RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla42
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
+++ This bug was initially created as a clone of Bug #1186875 +++

The root cause of bug 1186875 is that there is something going on with running gtk3 builds during the build that doesn't work properly, between pango and fontconfig (and in fact, likely both).

And we run builds during valgrind *and* PGO. Which means we're not PGOing as well as we should be, which likely explains the general regressions there were on linux PGO builds.
would this explain the dozen of pretty serious mostly pgo only regressions we see on Tuesday:
http://alertmanager.allizom.org:8080/alerts.html?rev=bf7d50538de6&showAll=1&testIndex=0&platIndex=0

Still working on generating data to ensure we have a full understanding of the differences.
(Assignee)

Comment 2

3 years ago
I wouldn't waste too much time on the PGO regressions if I were you. Let's see how things go once I figure out how to fix this bug, because this does need to be fixed somehow anyways. I'm more interested in the non-PGO regressions at the moment, but there doesn't seem to be a lot of them, and considering the change of toolkit, I'm a) not surprised and b) pretty sure there's nothing we can do.
(Assignee)

Updated

3 years ago
Depends on: 1187269
given the fact that we had a bug in talos when this landed, we don't have our fancy compare view, but we do have other tools to compare from the command line.

Here is the list of non-pgo  failures:
Linux:
:( ts_paint            1109.4 +/-  1% (7#)  [  +5.2%]    1166.6 +/-  0% (6#)
:) tscrollx              10.0 +/-  0% (12#)  [ -14.5%]       8.5 +/-  0% (12#)
:) tp5o_scroll            9.1 +/-  0% (7#)  [ -20.3%]       7.2 +/-  1% (6#)
:) tps                   40.6 +/-  1% (7#)  [  -2.0%]      39.8 +/-  4% (8#)

Linux64:
:( ts_paint            1051.5 +/-  1% (7#)  [  +4.1%]    1094.2 +/-  1% (6#)
:) tscrollx               9.0 +/-  1% (12#)  [ -16.8%]       7.5 +/-  1% (12#)
:( glterrain             22.0 +/-  1% (7#)  [  +3.5%]      22.8 +/-  9% (6#)
:) tp5o_scroll            9.0 +/-  1% (7#)  [ -16.6%]       7.5 +/-  7% (6#)

Win7:
:( tsvgr_opacity        272.8 +/-  1% (7#)  [  +7.0%]     292.0 +/- 21% (7#)


a mix of wins, losses.  For reference, I will post the pgo issues in my next comment
   test      Master     gmean  stddev points  change      gmean  stddev points  graph-url
Linux:
:( kraken              1600.6 +/-  0% (6#)  [  +2.3%]    1638.1 +/-  0% (6#)
:( v8_7               21223.0 +/-  0% (6#)  [  -7.7%]   19595.1 +/-  3% (6#)
:( dromaeo_css         5554.6 +/-  1% (6#)  [  -5.0%]    5276.1 +/-  1% (6#)
:( dromaeo_dom         1095.6 +/-  1% (6#)  [ -10.1%]     984.8 +/-  1% (6#)
:( a11yr                255.7 +/-  0% (6#)  [ +33.2%]     340.5 +/-  0% (6#)
:( ts_paint             928.9 +/-  0% (6#)  [ +11.7%]    1037.3 +/-  0% (6#)
:( tpaint               176.0 +/-  2% (6#)  [ +10.7%]     194.8 +/-  3% (6#)
:( tsvgr_opacity        178.1 +/- 10% (6#)  [ +21.4%]     216.2 +/-  0% (6#)
:( tp5o                 273.3 +/-  1% (6#)  [ +18.1%]     322.9 +/-  1% (6#)
:( tart                   7.2 +/-  1% (6#)  [ +16.9%]       8.4 +/-  0% (6#)
:( tsvgx                558.2 +/-  0% (6#)  [  +8.3%]     604.3 +/-  0% (6#)
:) tscrollx               9.0 +/-  0% (6#)  [  -9.5%]       8.1 +/-  0% (6#)
:( sessionrestore      1576.9 +/-  0% (6#)  [  +3.7%]    1634.6 +/-  1% (6#)
:( sessionrestore_no_auto_restore   668.8 +/-  1% (6#)  [  +4.2%]     696.9 +/-  0% (6#)
:( cart                  45.1 +/-  1% (6#)  [ +17.8%]      53.2 +/-  2% (6#)
:) tp5o_scroll            8.4 +/-  1% (6#)  [ -15.0%]       7.1 +/-  1% (6#)
:( damp                 547.8 +/-  1% (6#)  [  +6.6%]     583.9 +/-  0% (6#)
:( tps                   31.0 +/-  2% (6#)  [  +4.0%]      32.3 +/-  6% (6#)

Linux64:
:( tresize               15.6 +/-  2% (6#)  [ +12.6%]      17.6 +/-  1% (6#)
:( kraken              1456.9 +/-  0% (6#)  [  +2.1%]    1487.3 +/-  0% (6#)
:( dromaeo_css         6033.7 +/-  2% (6#)  [  -4.4%]    5769.7 +/-  2% (6#)
:( dromaeo_dom         1141.9 +/-  2% (6#)  [  -9.2%]    1037.2 +/-  1% (6#)
:( a11yr                203.2 +/-  0% (6#)  [ +37.2%]     278.7 +/-  1% (6#)
:( ts_paint             865.8 +/-  0% (6#)  [ +12.4%]     972.8 +/-  1% (6#)
:( tpaint               171.5 +/-  2% (6#)  [  +9.6%]     188.0 +/-  1% (6#)
:( tp5o                 245.1 +/-  1% (6#)  [ +14.1%]     279.7 +/-  2% (5#)
:( tart                   6.6 +/-  0% (6#)  [ +15.1%]       7.6 +/-  0% (6#)
:) tcanvasmark         6879.0 +/-  1% (6#)  [  -4.6%]    6560.1 +/-  1% (6#)
:( tsvgx                450.1 +/-  1% (6#)  [  +5.5%]     474.9 +/-  0% (6#)
:) tscrollx               7.9 +/-  1% (6#)  [ -12.4%]       6.9 +/-  2% (6#)
:( sessionrestore      1520.4 +/-  0% (6#)  [  +3.0%]    1566.2 +/-  0% (6#)
:( sessionrestore_no_auto_restore   623.8 +/-  1% (6#)  [  +3.2%]     644.0 +/-  1% (6#)
:( cart                  44.1 +/-  2% (6#)  [ +12.8%]      49.8 +/-  1% (6#)
:) tp5o_scroll            8.1 +/-  1% (6#)  [ -14.2%]       7.0 +/-  1% (6#)
:( damp                 505.5 +/-  0% (6#)  [  +5.6%]     533.9 +/-  1% (6#)
:( tps                   29.0 +/-  4% (6#)  [  +5.5%]      30.6 +/-  5% (6#)
(Assignee)

Comment 6

3 years ago
Not the final fix, but these talos numbers look like what they were before (but I only looked at a few)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a05421b7ad3
(Assignee)

Comment 7

3 years ago
Created attachment 8638968 [details] [diff] [review]
Set things up to use Gtk+3 from the tooltool package during PGO builds

The mk_add_options exports are meant to end up in $topobjdir/.mozconfig.mk,
which is included every time make runs.
Assignee: nobody → mh+mozilla
Attachment #8638968 - Flags: review?(gps)
(Assignee)

Comment 8

3 years ago
Created attachment 8638969 [details] [diff] [review]
Make .mozconfig.mk environment variables available to mach valgrind-test
Attachment #8638969 - Flags: review?(gps)
these changes do look to fix our pgo numbers, I am not sure if it is 100%, but it helps.

:glandium, have you looked at the non-pgo regressions?

Updated

3 years ago
Attachment #8638968 - Flags: review?(gps) → review+
Comment on attachment 8638969 [details] [diff] [review]
Make .mozconfig.mk environment variables available to mach valgrind-test

Review of attachment 8638969 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a fan of the pymake dependency. But, it's easy enough to extract that function to mozbuild.util in a follow-up, so I'm not too worried.
Attachment #8638969 - Flags: review?(gps) → review+
(Assignee)

Comment 11

3 years ago
(In reply to Joel Maher (:jmaher) from comment #9)
> these changes do look to fix our pgo numbers, I am not sure if it is 100%,
> but it helps.
> 
> :glandium, have you looked at the non-pgo regressions?

There's nothing we can do about them, short of switching back to Gtk+2.
I assume profiling would show in the code where we are taking more time and possible shed light on a fix?  We are talking about a huge drop in tp5_scroll and a noticeable drop in ts_paint.  I don't think accepting these without some kind of basic investigation seems reasonable.
(Assignee)

Comment 13

3 years ago
The huge drop in tp5_scroll is an *improvement*. And yes, scrolling is known to be faster with Gtk+3.
(Assignee)

Comment 14

3 years ago
(In reply to Gregory Szorc [:gps] from comment #10)
> Comment on attachment 8638969 [details] [diff] [review]
> Make .mozconfig.mk environment variables available to mach valgrind-test
> 
> Review of attachment 8638969 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not a fan of the pymake dependency. But, it's easy enough to extract
> that function to mozbuild.util in a follow-up, so I'm not too worried.

Note python/mozbuild/mozbuild/mach_commands.py already has a pymake dependency for the same command line parser (and bug 1151124 is opened related to it, and I'd argue that's where it could be moved to .util).
:glandium, thanks for pointing out the obvious- proof that I should not work so much as I mix things up!

ok, ts_paint is a regression, while it is important, I think accepting the wins in place is probably better.
https://hg.mozilla.org/mozilla-central/rev/ff5dbfee79ed
https://hg.mozilla.org/mozilla-central/rev/6f2cb2f8b5bc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42

Updated

6 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.