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

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
4 years ago
4 years ago

People

(Reporter: jmaher, Unassigned)

Tracking

({perf, regression})

unspecified
perf, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 affected)

Details

(Whiteboard: [talos_regression])

(Reporter)

Description

4 years ago
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.
(Reporter)

Comment 1

4 years ago
jdaggett, can you look into this?
Flags: needinfo?(jdaggett)
(Reporter)

Updated

4 years ago
Summary: 7% linux/osc v8 regression on inbound (v.35) Oct 1 from push 109ae9a694cc → 7% linux/osx v8 regression on inbound (v.35) Oct 1 from push 109ae9a694c

Comment 2

4 years ago
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.
Flags: needinfo?(jdaggett)
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

4 years ago
(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.
(Reporter)

Comment 5

4 years ago
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

4 years ago
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?
Flags: needinfo?(bzbarsky)
John, by default on Octane, higher scores are better. So the numbers you're reporting look like an improvement, not a regression.
I don't see how the font changes would have affected the v8 test.  We shouldn't be CCing in there, I'd think.
Flags: needinfo?(bzbarsky)

Comment 9

4 years ago
(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.
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?
Flags: needinfo?(continuation)
There shouldn't be any CC going on during splay.  If there is, we're doing it wrong.
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?
Flags: needinfo?(continuation) → needinfo?(jdemooij)
status-firefox35: --- → affected
tracking-firefox35: --- → +
(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...
Flags: needinfo?(jdemooij)
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.
(Reporter)

Comment 16

4 years ago
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!
Thanks for double checking, Joel.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WORKSFORME
tracking-firefox35: + → ---
You need to log in before you can comment on or make changes to this bug.