Closed Bug 1080630 Opened 6 years ago Closed 6 years ago
7% linux/osx v8 regression on inbound (v
.35) Oct 1 from push 109ae9a694c
We have some regressions from talos posted on dev.tree-management: http://126.96.36.199: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.
jdaggett, can you look into this?
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
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.
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.
(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.
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
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?
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.
(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.
this is the machines we use for testing: https://wiki.mozilla.org/Buildbot/Talos/Misc#Hardware_Profile_of_machines_used_in_automation
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?
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)
(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...
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.
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
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.