1378149 - Performance Regressions in IE Test Drive Benchmarks

Status: RESOLVED INCOMPLETE

(Core :: DOM: Core & HTML, defect, P3)

Description

[anleck]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170628075643

Steps to reproduce:

Ran Benchmarks:

https://testdrive-archive.azurewebsites.net/Performance/FishBowl/Default.html
https://testdrive-archive.azurewebsites.net/Performance/MrPotatoGun/Default.html

Noticed Nightly x64 performance was greatly reduced from that of 54.0.1 x64

Tested using clean profiles and Win x64 Zips - found the 06-May-2017 and 02-Jun-2017 55.0a1 builds to introduce performance drops.

All testing with x64 builds - may also present in x86


Actual results:

55.0a1

FishBowl (No. of fish - auto setting)
01-Jun  -  5,100
02-Jun  -  800


Mr. Potato Gun
05-May  -  40,000
06-May  -  22,000

01-Jun  -  22,000
02-Jun  -  12,000


Expected results:

54.0.1

FishBowl (No. of fish - auto setting)
5,100 Fish

Mr. Potato Gun
40,000 Score


Would expect parity (or improvement) of performance between Release and future builds.

Comment 1

[greg]

Can confirm this, also tested using clean profiles and Win x64 Zips:

nightly 04.07.2017
"buildID": "20170704030203",
"userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0"

MrPotatoGun: 26825
FishBowl: 1400

Firefox 54 with addons
"buildID": "20170704030203",
"userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0"

MrPotatoGun: 36624
FishBowl: 4000

Edge
Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36 Edge/15.15063

MrPotatoGun: 66732
FishBowl: 4804

nightly 29.06.2016
"buildID": "20160629030209",
"userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0",

MrPotatoGun: 37947
FishBowl: 5640

nightly 29 Jun 2014 x86
"userAgent": "Mozilla/5.0 (Windows NT 6.3; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0"

MrPotatoGun: 37835
FishBowl: 2811

Comment 2

[Marco Bonardo [:mak]]

could you please attach a log from about:support?

Moving to Graphics for now, IIRC these tests benefited from hw acceleration. I didn't profile it though (it's too fast on my system).

Comment 3

[anleck]

Attachment: Anleck - about:support

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

(In reply to anleck from comment #0)
> 05-May  -  40,000
> 06-May  -  22,000

"Speedy" Regression window (mozilla-central)
Good:
https://ftp.mozilla.org/pub/firefox/nightly/2017/05/2017-05-05-03-02-52-mozilla-central/

Bad:
https://ftp.mozilla.org/pub/firefox/nightly/2017/05/2017-05-06-03-02-04-mozilla-central/

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b255199db9d6a6f189b89b7906f99155bde3726&tochange=37a5b7f6f101df2eb292b1b6baaf1540c9920e20

Could be caused by (looking only on Core:Graphics bugs):
bug #1361257 [Core:Graphics: Layers]-Don't synchronously flush rendering on Windows
bug #1360478 [Core:Graphics: Layers]-Fix crash when initializing APZ after compositor shutdown
bug #1362145 [Core:Graphics: Layers]-Preloading DeviceAttachmentsD3D11 can race
bug #1362167 [Core:Graphics: Text]-Reduce size of gfxShapedWord and gfxTextRun classes by optimizing field sizes and arrangement
bug #1358502 [Core:Graphics: Text]-Update harfbuzz to version 1.4.6 [was: IPA Characters Overlap When Entering Text on 64-bit Windows systems]
bug #1362117 [Core:Graphics: Text]-cairo_font_face_t's created from UnscaledFontFontconfig should hold a reference to the NativeFontResourceFontconfig that created them
bug #1343754 [Core:Graphics]-gfx-labeling Label runnables in gfx rendering.
bug #1357307 [Core:Graphics]-startup crash in gfxPrefs::CMSMode() with session manager installed

so bug #1361257 or bug #1362145 look the most suspicious here



(In reply to anleck from comment #0)
> 01-Jun  -  22,000
> 02-Jun  -  12,000

"Speedy" Regression window (mozilla-central)
Good:
https://ftp.mozilla.org/pub/firefox/nightly/2017/06/2017-06-01-03-02-06-mozilla-central/

Bad:
https://ftp.mozilla.org/pub/firefox/nightly/2017/06/2017-06-02-03-02-04-mozilla-central/

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bdb2387396b4a74dfefb7c983733eed3625e906a&tochange=aeb3d0ca558f034cbef1c5a68bd07dd738611494

Could be caused by (looking only on Core:Graphics bugs):
bug #1364089 [Core:Graphics: Text]-Remove unnecessary virtual method calls in gfxFont/gfxFontGroup code
bug #1368593 [Core:Graphics: WebRender]-Do not set background color if WebRenderBridgeParent is not root
bug #1368190 [Core:Graphics: WebRender]-Enable outer box shadows with negative spread radius
bug #1368551 [Core:Graphics: WebRender]-Make layout/reftests/async-scrolling/split-opacity-layers-1.html pass on linux64-qr with APZ enabled
bug #1368483 [Core:Graphics: WebRender]-Pass WrColor in wr_api_set_root_display_list()
bug #1367734 [Core:Graphics: WebRender]-Update webrender to a54cc729259588dd1ff52c86d0c62cb2a1767137
bug #1368730 [Core:Graphics]-Assertion failure: !mTextureData in TextureChild::~TextureChild when GPU process aborts
bug #1369096 [Core:Graphics]-Check HRESULT when QueryInterfacing an ID2D1Image for an ID2D1Bitmap
bug #1369490 [Core:Graphics]-Remove some backend type checks that aren't needed

so bug #1369096 or bug #1369490 look the most suspicious here

Comment 5

[Arjen de Jager]

According to MozRegression Bug 1363829 caused this regression on Mr. Potato Gun:

2017-07-04T22:05:22: INFO : Narrowed inbound regression window from [1de33be1, 7c6116ac] (3 revisions) to [83f3573e, 7c6116ac] (2 revisions) (~1 steps left)
2017-07-04T22:05:22: DEBUG : Starting merge handling...
2017-07-04T22:05:22: DEBUG : Using url: https://hg.mozilla.org/integration/mozilla-inbound/json-pushes?changeset=7c6116ac71e954805dccf3146d6fffcf28bbc0cf&full=1
2017-07-04T22:05:22: DEBUG : Found commit message:
Bug 1363829 P17 Make browser_net_simple-request-data.js wait for all async tests to complete before finishing. r=honza

2017-07-04T22:05:22: INFO : The bisection is done.
2017-07-04T22:05:22: INFO : Stopped

On Fish Bowl I see an improvement instead of a regression. Maybe Bug 1363829 affects only Mister Potato Gun and Fish Bowl is eventually caused by a different change?

Comment 6

[Arjen de Jager]

Attachment: aboutsupport.txt

Comment 7

[anleck]

Post describing initial testing

http://forums.mozillazine.org/viewtopic.php?p=14754505#p14754505

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Let me check a few things here...

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

I believe the potato gun thing is due to bug 1378394.  That demo site runs its animation from:

  setInterval(Draw, 0);

Which should fire an interval every 4ms.  We accidentally changed how we set calculate the interval, however, so the animation is running slightly slower than it did before.

Comment 10

[anleck]

Win_x64 Zip
56.0a1 Build ID - 20170705094029

Fixes FishBowl, does not fix Mr. Potato Gun.

Moving the mouse throughout the benchmark raises Potato Gun performance to Release (54.0.1) level.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

I have a potential fix for the potato gun thing here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54726295515f7e788ba41f1502fcc9890157034d&selectedJob=111977867

Does this try build improve the situation on your machine at all?

https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-54726295515f7e788ba41f1502fcc9890157034d/try-win64/

Comment 12

[anleck]

Same build as tried above.  FishBowl is better upto 5,480 - 5,500 mark (best ever was 6,000 - that was a very long time ago, years i think).

Comment 13

[greg]

At my PC, see Post 1 - https://bugzilla.mozilla.org/show_bug.cgi?id=1378149#c1

MrPotatoGun: 37500
FishBowl: 5100 ~ 5400 (fluctuating)

Comment 14

[Arjen de Jager]

(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #11)
> I have a potential fix for the potato gun thing here:
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=54726295515f7e788ba41f1502fcc9890157034d&selectedJob=1
> 11977867
> 
> Does this try build improve the situation on your machine at all?
> 
> https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-
> 54726295515f7e788ba41f1502fcc9890157034d/try-win64/

It seems to improve the situation, but not very much.

06-01: 44084/43984/44081
06-02: 25875/23721/23841/26035/18159/23763
Try:   26974/27147/27017

I don't know why the results of 06-02 fluctuate significantly more compared to 06-01 and Try. However, IMHO a slight improvement.

Comment 15

[Ben Kelly [:bkelly, not reviewing]]

I think its likely some of the MrPotatoGun regression is because we fixed a bug.  Consider this:

  var lastTime = performance.now();
  function interval() { let start = performance.now(); console.log(start - lastTime); lastTime = start; }
  var id = setInterval(interval, 0);

On Firefox 54 this gives unexpected values like:

             4.6450000000040745 
             4.989999999997963 
             5.010000000002037 
             4.989999999997963 
Too fast --> 0.2900000000008731 
             4.709999999999127 
             5.05000000000291 
             5.024999999994179 
             5.014999999999418 
Too fast --> 0.3150000000023283 
             4.614999999997963

We always clamp setInterval() to a minimum of 4ms, but in FF54 we are firing after 0ms every fifth callback.  This was a bug and has been fixed in FF55+.  Since MrPotatoGun is run from setInterval(f, 0), it was most likely benefiting from this unintended behavior.

Comment 16

[anleck]

56.0a1 Build ID - 20170705094029
Did some more poking (with that same try build).

Resized the browser window to a little less than full screen:

Ran Mr P_Gun - no change, performance low.

Tried running again moving the mouse cursor within the browser window - performance up to Release level (stopping the mouse at any point will result in slow down, moving again will pick it up).

Ran again moving the cursor outside of the browser window - no effect, performance low.

Comment 17

[Ben Kelly [:bkelly, not reviewing]]

The MrPotatoGun site seems to mainly be testing how fast we fire setInterval() callbacks.  Its not really limited by drawing at all.  I took a profile and its almost completely idle.

Also, you can greatly change the fps/score values by simplying changing this pref in about:config:

  dom.min_timeout_value

While this is interesting, I'm not sure MrPotatoGun is worth optimizing for.  We need to have the flexibility to slow down things like setInterval(f, 0) to improve overall browser performance.  We don't really guarantee accurate callbacks faster than 60fps.  And its better to use requestAnimationFrame() to get that.

Comment 18

[Ben Kelly [:bkelly, not reviewing]]

Even if you call EnableTurboMode() to fire faster than the setInterval() the main thread is still 60% idle on my windows 10 machine.

Comment 19

[anleck]

Thanks Ben.

I have no idea why browser window interaction is neccessary for that test to run properly on those 55+ builds for me (yea, it's not taking full advantage of the hardware). I have tried the first 52 build I could get a zip of and it runs well - don't know if that is helpful at all.
Unfortunately I don't have another system I can test with.

I agree it is not worth investing too much effort on this if it does not have impact elsewhere, or expose a deeper issue.

Comment 20

[anleck]

Further testing.

The initial maybe half second runs fast before performance drops.

Mouse cursor must move within the 'painted window'.  If I start the test then open the hambuger menu, running the cursor along the menu does not help.

Having another browser window open to http://forums.mozillazine.org/ (I used Release - another window opened from the same try build did not work) brings performance up to Release level, staying at about:home did not.

Comment 21

[anleck]

Mouse cursor movement within the 'painted window' effect - can also be seen @ https://www.vsynctester.com/
54.0.1 also displays this behaviour at this site.

Comment 22

[greg]

For my Enviroment its fixed with the try build.
I didnt moved the mouse cursor after the Tests started.

Comment 23

[anleck]

Interesting, thank-you Greg.
Try build fixed FishBowl (actually better now than release) and I have noticed improvment elsewhere too.  This Try build is probably the best build I have used overall.

This patch should make release.

Should have some more time later.  Will try reinstall of nvidia drivers.

Comment 24

[anleck]

Re-downloaded Try build, new clean profile, re-install of nvidia drivers - No difference.

Also noticed can't play video from https://twit.tv/ with the Try build.

Comment 25

[anleck]

Sorry Nightly itself appears to have broken video playback at twit.tv

Comment 26

[anleck]

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

Patch seems to work well

Comment 27

[Hsin-Yi Tsai (she/her) [:hsinyi]]

According to comment 19 and it's late at 55 beta cycle, I am thinking of setting firefox55 fix-optional. If bug 1378586 does help here, we could consider uplift. Sounds right?

Comment 28

[Ben Kelly [:bkelly, not reviewing]]

I'm not really sure the state of this bug.  It reported problems on two different sites which seem to be due to different issues.

The MrPotatoGun site helped me find a setInterval() bug that has been fixed.  Otherwise I don't see much else to fix.  The site is not actually hitting any graphics/perf issues.  The number is purely an artifact of setInterval() scheduling now and we're not interested in changing it just for this artificial metric.

I'm unsure what the case is for the other site.  It seems that may be a legit gfx issue, but I'm not sure.

Comment 29

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 30

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Moving open bugs with topperf keyword to triage queue so they can be reassessed for performance priority.

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

According to comment 28, we can close this bug. This is a performance regression report 6 years ago, therefore, I think that it does not make sense to keep handling this as a performance regression bug since there must be too many changes after that. If there is a performance issue in the benchmarks, it should be filed separately.

Comment 33

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I agreed with comment 32.