1080630 - 7% linux/osx v8 regression on inbound (v.35) Oct 1 from push 109ae9a694c

Status: RESOLVED WORKSFORME

(Core :: DOM: CSS Object Model, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

We have some regressions from talos posted on dev.tree-management:
http://54.215.155.53:8080/alerts.html?rev=109ae9a694cc&table=1

and here are some graphs:
http://graphs.mozilla.org/graph.html#tests=%5B%5B230,131,33%5D,%5B230,131,35%5D,%5B230,63,24%5D,%5B230,63,21%5D%5D&sel=1410184019583,1412776019583&displayrange=30&datatype=running


I did some retriggers:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=0b4ff0427b86&tochange=0f14e8b69354&jobname=mozilla-inbound%20talos%20dromaeojs

and you can see the difference here:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=109ae9a694cc

Since :heycam is gone for the next month (it was unfortunate that this regression detection system for talos hung for a few days and we got our regressions a week later), we need someone to address this.  It appears that jdaggett did most of the reviews.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

jdaggett, can you look into this?

Comment 2

[John Daggett (:jtd)]

Joel, I think it's probably better for someone on the JS team to look at this. The V8 test doesn't do any font loading so I'm at a loss to understand how those patches could have affected V8 perf. If someone from the JS can point me to an underlying reason for the regression, I can work on how to fix whatever it is within the font loading code that caused that. Right now I somewhat skeptical that the changes Cam landed were actually the cause.

Comment 3

[Cameron McCormack (:heycam)]

Can we say printf something every time FontFaceSet::UpdateRules gets run, run the test with that, then do the same with a printf in the old nsUserFontSet::UpdateRules in the revision before the patch queue landed?  That ought to tell us whether we're doing more user font set work.

Comment 4

[John Daggett (:jtd)]

(In reply to Cameron McCormack (:heycam) (away October 6 - November 5) from comment #3)
> Can we say printf something every time FontFaceSet::UpdateRules gets run,
> run the test with that, then do the same with a printf in the old
> nsUserFontSet::UpdateRules in the revision before the patch queue landed? 
> That ought to tell us whether we're doing more user font set work.

We already have logging for just that sort of thing :)

That's what I did to check that fonts were not be loaded, I ran the test with 'userfonts' logging enabled.

  NSPR_LOG_MODULES=userfonts:5

No userfonts added or loaded while running V8.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

if you can get this to load:
https://datazilla.mozilla.org/?start=1411704464&stop=1412225631&product=Firefox&repository=Mozilla-Inbound-Non-PGO&os=linux&os_version=Ubuntu%2012.04&test=v8_7&graph_search=109ae9a694cc&x86_64=false&project=talos

you will see Splay and Boyer are the two tests that have regressed:
http://hg.mozilla.org/build/talos/file/b2a3d0493173/talos/page_load_test/v8_7/earley-boyer.js
http://hg.mozilla.org/build/talos/file/b2a3d0493173/talos/page_load_test/v8_7/splay.js

this is caused by the patches that landed as mentioned above.  We can ask the JS guys for help, but with a >5% regression on 4 platforms we really need to figure out the root cause and either accept it with documentation, back this out, or reduce the severity with some fixes.

here are some steps on how to run talos locally:
https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code

Comment 6

[John Daggett (:jtd)]

Using the V8 benchmark, v7:
http://v8.googlecode.com/svn/data/benchmarks/v7/run.html

Nightly 2014-09-26 (OSX 10.8)
splay 10700 / 10561 / 10928 avg: 10730

Nightly 2014-10-13 (OSX 10.8)
splay 11352 / 11306 / 11187 avg: 11282 (-5.1%)

bz, I'm at a loss to understand how the patches on bug 1028497 could have affected JS perf like this. As in previous comments, there's no use of this API or of downloadable fonts in these tests (the pref was off in the profile I used).

The only thing I can think of is maybe this somehow affected cycle collection adversely?

Comment 7

[Emanuel Hoogeveen [:ehoogeveen]]

John, by default on Octane, higher scores are better. So the numbers you're reporting look like an improvement, not a regression.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I don't see how the font changes would have affected the v8 test.  We shouldn't be CCing in there, I'd think.

Comment 9

[John Daggett (:jtd)]

(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #7)
> John, by default on Octane, higher scores are better. So the numbers you're
> reporting look like an improvement, not a regression.

Ok, this is getting stranger and stranger. The graph server clearly shows some sort of regression. What's the environment for those? Mac mini's still? Maybe this is some sort of physical memory related effect?

Since the splay test claims to be testing automatic memory management, I suspect this is cycle collector related, as that's the only part of this code that relates in any way to performance.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

this is the machines we use for testing:
https://wiki.mozilla.org/Buildbot/Talos/Misc#Hardware_Profile_of_machines_used_in_automation

Comment 11

[Jet Villegas (inactive)]

It's quite odd that we have a regression in a JS test with an unrelated feature (that's preffed off!) 

The only diff I can see that might still be web-facing despite the feature turned off is this DOM change:

https://hg.mozilla.org/integration/mozilla-inbound/diff/52e4991a8216/dom/webidl/Document.webidl

Would this affect CC perf per comment 9?

Comment 12

[Boris Zbarsky [:bzbarsky]]

There shouldn't be any CC going on during splay.  If there is, we're doing it wrong.

Comment 13

[Andrew McCreight [:mccr8]]

I've never seen any CC change have any effect on any v8 benchmark.  v8 benchmarks run the GC a lot, not the CC.

Jan do you have any suggestions on how to go about investigating this?

Comment 14

[Jan de Mooij [:jandem]]

(In reply to Andrew McCreight [:mccr8] from comment #13)
> I've never seen any CC change have any effect on any v8 benchmark.  v8
> benchmarks run the GC a lot, not the CC.
> 
> Jan do you have any suggestions on how to go about investigating this?

Unfortunately no. I tried to repro this locally on OS X but failed. We also don't see a regression on AWFY for Octane in either the shell or browser...

Comment 15

[Cameron McCormack (:heycam)]

When I landed bug 1092570, which stopped accidentally exposing FontFace(Set) constructors on the global, we got a V8 improvement.

https://groups.google.com/forum/#!searchin/mozilla.dev.tree-management/Avoid$20exposing$20FontFace/mozilla.dev.tree-management/26nm3Uwghbc/GNpEPonRTE0J

The notification email did include a few other bugs, so can't be sure, but this might've been it.

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

if you pull up the original graph linked and adjust it to 90 days, you will see the longer trend:
http://graphs.mozilla.org/graph.html#tests=%5B%5B230,131,33%5D,%5B230,63,24%5D,%5B230,131,35%5D,%5B230,63,21%5D%5D&sel=none&displayrange=90&datatype=running

in this case it appears we have fixed the regressions (higher is better) on all platforms!  I am not sure if it is related to this specifically, but either way we will uplift to aurora with no regression which is a good thing.

Feel free to close this!

Comment 17

[Cameron McCormack (:heycam)]

Thanks for double checking, Joel.