Closed Bug 1419079 Opened 7 years ago Closed 7 years ago

High CPU Firefox 57 & 59 on Mac: www.theguardian.com/uk, Firefox 52 ESR has low CPU

Categories

(Core :: DOM: Animation, defect, P2)

Product:
Core
Component:
DOM: Animation
Version:
55 Branch
Platform:
Unspecified
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: mark.paxman99, Assigned: hiro)

References

(Blocks 1 open bug)

Details

(Keywords: perf, power, regression)

Attachments

(6 files)

News, sport and opinion from the Guardian's UK edition | The Guardian.html
(deleted), text/html
Details
Bug 1419079 - Drop checking the pref for animations-api core in file_restyles.html.
(deleted), text/x-review-board-request
birtles
: review+
Details
Bug 1419079 - Drop checking the pref for offscreen throttling.
(deleted), text/x-review-board-request
birtles
: review+
Details
Bug 1419079 - Revise a comment for a test case for missing keyframes.
(deleted), text/x-review-board-request
birtles
: review+
Details
Bug 1419079 - Add a test case for additive animation with a missing keyframe.
(deleted), text/x-review-board-request
birtles
: review+
Details
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'.
(deleted), text/x-review-board-request
birtles
: review+
Details 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20171120100042

Steps to reproduce:

https://www.theguardian.com/uk    on MacOS 10.13.1

has very high CPU load with Firefox 57 & 59 but low CPU with Firefox 52. Servo on/off makes no difference. The CPU load seems to stay high indefinitely. 
								App			Web Content
Firefox ESR 52.4.0					~5%			~5%
Firefox 57.0 servo ON or OFF			~8%			~60%    <------
Firefox 59 servo ON					~8%			~60%    <------

Chrome  ~5%
Safari ~1%

Something to do with the two animated slideshow panels, it seems horrible, like there are four overlapping animations in each slideshow and so Firefox is animating lots of stuff all the time. But I am no expert.
Component: Untriaged → General
Product: Firefox → Core
The page seems OK today. I will upload a saved version of the high CPU page. The slideshow animated images (Mugabe, the fashion designer) seem to be the problem, I zapped them using uBlock Origin and CPU levels dropped dramatically. But the slideshows don't seem to be on today's page.

(If you turn WebRender on the CPU load on the uploaded page is horrible, ~100% for Nightly + ~100% for content)
Mark, can you see if safe mode helps at all?

https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode
Flags: needinfo?(mark.paxman99)
Safe Mode crashes on Nightly, I just filed bug 1419504   :)

I tried Safe Mode on Firefox 57 and no change, still very high CPU load.

On that web archive which I uploaded (attachment 8930451 [details]):- 

~90% (web content process) + ~30% (Firefox process), Safe Mode on or off


I downloaded an Add-on called No Transition which kills animations. Using that add-on stops the slideshow (Mugabe etc) and CPU drops dramatically. Zapping the slideshows with uBlock Origin has the same effect.

(Overall Quantum does seem to use a lot of CPU for animations, there are other sites I am struggling with eg www.theverge.com)
Flags: needinfo?(mark.paxman99)
Brian, is there anything you can recommend to check for high cpu usage during animations mentioned in comment 4?
Flags: needinfo?(bbirtles)
Mark, in firefox 57, can you disable your other addons and then install this addon:

https://perf-html.io/

And record a profile on the site you mention in comment 4?

More info about using the profiler addon is here:

https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler
Flags: needinfo?(mark.paxman99)
We've heard a few reports of high CPU usage on OSX in 57 onwards, not sure if animations have been involved in the other cases though.

The animations in the slideshow appear totally sane to me. On Windows I see them all running on the compositor.

The easiest way is to tell if animations are running on the compositor is in the Animations panel of the inspector. If the animation has the lightning bolt next to it, it should be running on the compositor (and it tends to be conservative in that it sometimes might not have the lightning bolt, even if it eventually does run on the compositor).

We've had a few cases of high CPU usage for invisible animations (since we can't run them on the compositor). Hiro has fixed a number of them but I believe we still don't optimize animations on elements that are visibility:hidden (bug 1237454) and there are probably a few other cases too (e.g. empty SVG paths etc.). However, I don't see that in this case -- the slideshow is clearly visible and the animations are clearly running on the compositor.

Just using the "Performance" panel of the DevTools I see some setTimeout traffic but I'm not sure if that's related.

Using the profiler on Nightly I see the content process gets busy at the same time the compositor does (i.e. when the fade between images takes place). On the content process I see a bit of work in display list building, and more work in layer building which surprises me because I assumed these animations would run entirely on the compositor without needing any work on the content side. Then again, this is on Nightly with retained display lists enabled, and on Windows too so it's probably completely different to what the reporter is seeing.
Flags: needinfo?(bbirtles)
FWIW, the perf-html.io addon works with full symbols on release channel now if you have time to look at it there as well.
Here's what I see on Windows on 57 with a fairly clean profile during the slideshow transition: https://perfht.ml/2B27MjT

There's a bit of display list building and layer building going on. Normally when we run animations on the compositor we don't need to do restyles on the main thread at all but it looks like script is triggering restyles every 100ms or so:

* https://pagead2.googlesyndication.com/pagead/js/lidar.js is fetching the computed opacity
* https://pagead2.googlesyndication.com/pagead/osd.js is fetching scrollLeft

The setTimeout calls appear to come from:
* https://s0.2mdn.net/879366/express_html_inpage_rendering_lib_200_212.js

I wonder if script triggers restyles (that otherwise would be throttled) and so we notice that the opacity has changed and go ahead and paint. Even so, I don't see why that would have changed between 52 and 57.

There's a tiny bit of intersection observer work (v55 onwards) but nothing that could explain 55% increase in CPU usage.

Again, though, this is on Windows.
I wonder if there is something different on Mark's machines compared to where you are profiling.

Mark, can you open "about:support" in a new tab, copy text to clipboard, and then paste here?
I plan to dig into this a lot more later today.

In the meantime, could you ask one of your Mac using colleagues to open attachment 8930451 [details] and check Activity Monitor?  That seems a simple way to see if the problem is just me or all Macs.
Flags: needinfo?(mark.paxman99)
There is a MacBook Pro running OS X 10.12.4 in the office here but I only see about 7~8% for Firefox and about 5% for Firefox CP Web Content. Then again, I was unable to reproduce the high CPU usage on cnn.com (bug 1414943) using that same MacBook too (while a friend on OSX 10.12.3 can).
(In reply to Mark from comment #11)
> I plan to dig into this a lot more later today.
> 
> In the meantime, could you ask one of your Mac using colleagues to open
> attachment 8930451 [details] and check Activity Monitor?  That seems a
> simple way to see if the problem is just me or all Macs.

I found there are 6 opacity animations that don't run on the compositor.  So I think they consume CPU.  But I have no idea yet why they don't run on the compositor.
(In reply to Hiroyuki Ikezoe (:hiro) (PTO on 11/24) from comment #13)
> (In reply to Mark from comment #11)
> > I plan to dig into this a lot more later today.
> > 
> > In the meantime, could you ask one of your Mac using colleagues to open
> > attachment 8930451 [details] and check Activity Monitor?  That seems a
> > simple way to see if the problem is just me or all Macs.
> 
> I found there are 6 opacity animations that don't run on the compositor.  So
> I think they consume CPU.  But I have no idea yet why they don't run on the
> compositor.

Oh, I guess they are on out-of-view element, so it should not consume CPU.  We need an indicator on devtools to represent that the animation is throttled.
I've got some Macs here of various vintages. 

In a nutshell, FF57 + attachment 8930451 [details] has high CPU all three of my Macs which are new enough to run FF57. Where I can use older versions of FF the CPU is much lower.

For each machine I
- created a new user
- downloaded a clean copy of FF57
- installed FF57 onto the desktop of the new user
- ran FF57 from the desktop
- loaded attachment 8930451 [details] as the only tab
- waited about a minute for Firefox to finish its housekeeping
- scrolled the page right up and down to fire any scripts.
- watched Activity Monitor for CPU load to settley

Results

High CPU:- ~50% (web content) + ~20% (Firefox)

    Retina MB Pro 2012 running MacOS 10.13.1 & clean FF57
    MB Air ~2014 running MacOS 10.13.1 & clean FF57
    MacBook ~2012 running MacOS 10.12.4 & clean FF57

Low CPU: ~10% for the whole of FF + web content

    Retina MB Pro 2012 running MacOS 10.13.1 & FF52 ESR
    MBP ~2006 running MacOS 10.6.8 & FF48

I'll focus on FF57 on my main rMBP and post some of the about:support stuff and the perf-html stuff you mention above.
OK Firefox 57 on my main rMBP
- refreshed Firefox
- disabled add ons except Gecko Profiler
- loaded attachment 8930451 [details]
- scrolled up and down the page
- waited a minute or so
- ran perf.html add on for about 20 seconds
- got a very hot computer ;)

https://perfht.ml/2iGW9bo

I have no idea what it means!




about.support is below. I have tinkered with about:config in the past but only on Nightly rarely on FF57.



{
  "application": {
    "name": "Firefox",
    "osVersion": "Darwin 17.2.0",
    "version": "57.0",
    "buildID": "20171112125346",
    "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0",
    "safeMode": false,
    "updateChannel": "release",
    "supportURL": "https://support.mozilla.org/1/firefox/57.0/Darwin/en-GB/",
    "numTotalWindows": 1,
    "numRemoteWindows": 1,
    "remoteAutoStart": true,
    "currentContentProcesses": 3,
    "maxContentProcesses": 4,
    "autoStartStatus": 1,
    "styloBuild": true,
    "styloDefault": true,
    "styloResult": true,
    "keyGoogleFound": true,
    "keyMozillaFound": true
  },
  "modifiedPreferences": {
    "browser.cache.disk.filesystem_reported": 1,
    "browser.cache.disk.smart_size.first_run": false,
    "browser.cache.frecency_experiment": 3,
    "browser.cache.disk.capacity": 358400,
    "browser.places.smartBookmarksVersion": 8,
    "browser.sessionstore.upgradeBackup.latestBuildID": "20171112125346",
    "browser.startup.homepage_override.mstone": "57.0",
    "browser.startup.homepage": "about:newtab",
    "browser.startup.homepage_override.buildID": "20171112125346",
    "browser.tabs.warnOnClose": false,
    "browser.tabs.remote.autostart.2": true,
    "browser.tabs.warnOnOpen": false,
    "browser.urlbar.lastSuggestionsPromptDate": 20171122,
    "browser.urlbar.daysBeforeHidingSuggestionsPrompt": 3,
    "browser.urlbar.usepreloadedtopurls.enabled": false,
    "browser.urlbar.searchSuggestionsChoice": true,
    "browser.urlbar.userMadeSearchSuggestionsChoice": true,
    "dom.ipc.processCount.web": 4,
    "extensions.lastAppVersion": "57.0",
    "font.internaluseonly.changed": false,
    "media.gmp-manager.lastCheck": 1511355064,
    "media.gmp.storage.version.observed": 1,
    "media.gmp-manager.buildID": "20171107091003",
    "network.cookie.cookieBehavior": 1,
    "network.predictor.cleaned-up": true,
    "network.cookie.prefsMigrated": true,
    "places.history.expiration.transient_current_max_pages": 104858,
    "plugin.disable_full_page_plugin_for_types": "application/pdf",
    "plugin.state.flip4mac wmv plugin": 0,
    "privacy.trackingprotection.introCount": 20,
    "privacy.trackingprotection.enabled": true,
    "security.sandbox.content.tempDirSuffix": "{3889564c-2ccd-a746-9c05-5747903d6947}",
    "services.sync.declinedEngines": "passwords",
    "services.sync.lastPing": 1511354643,
    "services.sync.lastSync": "Wed Nov 22 2017 12:56:40 GMT+0000 (GMT)",
    "services.sync.numClients": 4,
    "services.sync.engine.creditcards.available": true,
    "services.sync.engine.bookmarks.validation.lastTime": 1511354711,
    "services.sync.engine.addresses": true,
    "services.sync.engine.prefs.modified": false,
    "services.sync.engine.passwords": false
  },
  "lockedPreferences": {},
  "media": {
    "currentAudioBackend": "audiounit",
    "currentMaxAudioChannels": 2,
    "currentPreferredChannelLayout": "stereo",
    "currentPreferredSampleRate": 44100,
    "audioOutputDevices": [
      {
        "name": "Internal Speakers",
        "groupId": "AppleHDAEngineOutput:1B,0,1,1:0",
        "vendor": "Apple Inc.",
        "type": 2,
        "state": 2,
        "preferred": 15,
        "supportedFormat": 12336,
        "defaultFormat": 4096,
        "maxChannels": 2,
        "defaultRate": 44100,
        "maxRate": 96000,
        "minRate": 44100,
        "maxLatency": 4376,
        "minLatency": 294
      }
    ],
    "audioInputDevices": [
      {
        "name": "Internal Microphone",
        "groupId": "AppleHDAEngineInput:1B,0,1,0:1",
        "vendor": "Apple Inc.",
        "type": 1,
        "state": 2,
        "preferred": 15,
        "supportedFormat": 12336,
        "defaultFormat": 4096,
        "maxChannels": 2,
        "defaultRate": 44100,
        "maxRate": 96000,
        "minRate": 44100,
        "maxLatency": 4162,
        "minLatency": 80
      }
    ]
  },
  "javaScript": {
    "incrementalGCEnabled": true
  },
  "accessibility": {
    "isActive": false,
    "forceDisabled": 0,
    "handlerUsed": false,
    "instantiator": ""
  },
  "libraryVersions": {
    "NSPR": {
      "minVersion": "4.17",
      "version": "4.17"
    },
    "NSS": {
      "minVersion": "3.33",
      "version": "3.33"
    },
    "NSSUTIL": {
      "minVersion": "3.33",
      "version": "3.33"
    },
    "NSSSSL": {
      "minVersion": "3.33",
      "version": "3.33"
    },
    "NSSSMIME": {
      "minVersion": "3.33",
      "version": "3.33"
    }
  },
  "userJS": {
    "exists": false
  },
  "crashes": {
    "submitted": [],
    "pending": 0
  },
  "sandbox": {
    "contentSandboxLevel": 3,
    "effectiveContentSandboxLevel": 3
  },
  "graphics": {
    "numTotalWindows": 1,
    "numAcceleratedWindows": 1,
    "windowLayerManagerType": "OpenGL",
    "windowLayerManagerRemote": true,
    "windowUsingAdvancedLayers": false,
    "adapterDescription": "",
    "adapterVendorID": "0x8086",
    "adapterDeviceID": "0x0a2e",
    "adapterRAM": "",
    "adapterDrivers": "",
    "driverVersion": "",
    "driverDate": "",
    "offMainThreadPaintEnabled": false,
    "webgl1Renderer": "Intel Inc. -- Intel Iris OpenGL Engine",
    "webgl1Version": "4.1 INTEL-10.28.29",
    "webgl1DriverExtensions": "GL_ARB_blend_func_extended GL_ARB_draw_buffers_blend GL_ARB_draw_indirect GL_ARB_ES2_compatibility GL_ARB_explicit_attrib_location GL_ARB_gpu_shader_fp64 GL_ARB_gpu_shader5 GL_ARB_instanced_arrays GL_ARB_internalformat_query GL_ARB_occlusion_query2 GL_ARB_sample_shading GL_ARB_sampler_objects GL_ARB_separate_shader_objects GL_ARB_shader_bit_encoding GL_ARB_shader_subroutine GL_ARB_shading_language_include GL_ARB_tessellation_shader GL_ARB_texture_buffer_object_rgb32 GL_ARB_texture_cube_map_array GL_ARB_texture_gather GL_ARB_texture_query_lod GL_ARB_texture_rgb10_a2ui GL_ARB_texture_storage GL_ARB_texture_swizzle GL_ARB_timer_query GL_ARB_transform_feedback2 GL_ARB_transform_feedback3 GL_ARB_vertex_attrib_64bit GL_ARB_vertex_type_2_10_10_10_rev GL_ARB_viewport_array GL_EXT_debug_label GL_EXT_debug_marker GL_EXT_framebuffer_multisample_blit_scaled GL_EXT_texture_compression_s3tc GL_EXT_texture_filter_anisotropic GL_EXT_texture_sRGB_decode GL_APPLE_client_storage GL_APPLE_container_object_shareable GL_APPLE_flush_render GL_APPLE_object_purgeable GL_APPLE_rgb_422 GL_APPLE_row_bytes GL_APPLE_texture_range GL_ATI_texture_mirror_once GL_NV_texture_barrier",
    "webgl1Extensions": "ANGLE_instanced_arrays EXT_blend_minmax EXT_color_buffer_half_float EXT_frag_depth EXT_sRGB EXT_shader_texture_lod EXT_texture_filter_anisotropic EXT_disjoint_timer_query OES_element_index_uint OES_standard_derivatives OES_texture_float OES_texture_float_linear OES_texture_half_float OES_texture_half_float_linear OES_vertex_array_object WEBGL_color_buffer_float WEBGL_compressed_texture_s3tc WEBGL_compressed_texture_s3tc_srgb WEBGL_debug_renderer_info WEBGL_debug_shaders WEBGL_depth_texture WEBGL_draw_buffers WEBGL_lose_context MOZ_WEBGL_lose_context MOZ_WEBGL_compressed_texture_s3tc MOZ_WEBGL_depth_texture",
    "webgl1WSIInfo": "CGL",
    "webgl2Renderer": "Intel Inc. -- Intel Iris OpenGL Engine",
    "webgl2Version": "4.1 INTEL-10.28.29",
    "webgl2DriverExtensions": "GL_ARB_blend_func_extended GL_ARB_draw_buffers_blend GL_ARB_draw_indirect GL_ARB_ES2_compatibility GL_ARB_explicit_attrib_location GL_ARB_gpu_shader_fp64 GL_ARB_gpu_shader5 GL_ARB_instanced_arrays GL_ARB_internalformat_query GL_ARB_occlusion_query2 GL_ARB_sample_shading GL_ARB_sampler_objects GL_ARB_separate_shader_objects GL_ARB_shader_bit_encoding GL_ARB_shader_subroutine GL_ARB_shading_language_include GL_ARB_tessellation_shader GL_ARB_texture_buffer_object_rgb32 GL_ARB_texture_cube_map_array GL_ARB_texture_gather GL_ARB_texture_query_lod GL_ARB_texture_rgb10_a2ui GL_ARB_texture_storage GL_ARB_texture_swizzle GL_ARB_timer_query GL_ARB_transform_feedback2 GL_ARB_transform_feedback3 GL_ARB_vertex_attrib_64bit GL_ARB_vertex_type_2_10_10_10_rev GL_ARB_viewport_array GL_EXT_debug_label GL_EXT_debug_marker GL_EXT_framebuffer_multisample_blit_scaled GL_EXT_texture_compression_s3tc GL_EXT_texture_filter_anisotropic GL_EXT_texture_sRGB_decode GL_APPLE_client_storage GL_APPLE_container_object_shareable GL_APPLE_flush_render GL_APPLE_object_purgeable GL_APPLE_rgb_422 GL_APPLE_row_bytes GL_APPLE_texture_range GL_ATI_texture_mirror_once GL_NV_texture_barrier",
    "webgl2Extensions": "EXT_color_buffer_float EXT_texture_filter_anisotropic EXT_disjoint_timer_query OES_texture_float_linear WEBGL_compressed_texture_s3tc WEBGL_compressed_texture_s3tc_srgb WEBGL_debug_renderer_info WEBGL_debug_shaders WEBGL_lose_context MOZ_WEBGL_lose_context MOZ_WEBGL_compressed_texture_s3tc",
    "webgl2WSIInfo": "CGL",
    "info": {
      "AzureCanvasBackend": "skia",
      "AzureFallbackCanvasBackend": "none",
      "AzureContentBackend": "skia",
      "AzureCanvasAccelerated": 1,
      "ApzWheelInput": 1,
      "ApzDragInput": 1,
      "ApzKeyboardInput": 1,
      "ApzAutoscrollInput": 1,
      "TileHeight": 1024,
      "TileWidth": 1024
    },
    "featureLog": {
      "features": [
        {
          "name": "HW_COMPOSITING",
          "description": "Compositing",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        },
        {
          "name": "OPENGL_COMPOSITING",
          "description": "OpenGL Compositing",
          "status": "available",
          "log": [
            {
              "type": "default",
              "status": "available"
            }
          ]
        },
        {
          "name": "WEBRENDER",
          "description": "WebRender",
          "status": "unavailable",
          "log": [
            {
              "type": "default",
              "status": "opt-in",
              "message": "WebRender is an opt-in feature"
            },
            {
              "type": "runtime",
              "status": "unavailable",
              "message": "Build doesn't include WebRender"
            }
          ]
        }
      ],
      "fallbacks": []
    },
    "crashGuards": []
  },
  "experiments": [],
  "extensions": [
    {
      "name": "Gecko Profiler",
      "version": "0.17",
      "isActive": true,
      "id": "geckoprofiler@mozilla.com"
    },
    {
      "name": "No Transition",
      "version": "1.1.8",
      "isActive": false,
      "id": "{8b5fde66-c64d-4a33-99f1-c7c94138d67e}"
    },
    {
      "name": "uBlock Origin",
      "version": "1.14.18",
      "isActive": false,
      "id": "uBlock0@raymondhill.net"
    }
  ],
  "features": [
    {
      "name": "Activity Stream",
      "version": "2017.11.07.1100-7f4e3634",
      "id": "activity-stream@mozilla.org"
    },
    {
      "name": "Application Update Service Helper",
      "version": "2.0",
      "id": "aushelper@mozilla.org"
    },
    {
      "name": "Disable Media WMF NV12 format",
      "version": "1.1",
      "id": "disable-media-wmf-nv12@mozilla.org"
    },
    {
      "name": "Firefox Screenshots",
      "version": "19.2.0",
      "id": "screenshots@mozilla.org"
    },
    {
      "name": "Follow-on Search Telemetry",
      "version": "0.9.6",
      "id": "followonsearch@mozilla.com"
    },
    {
      "name": "Form Autofill",
      "version": "1.0",
      "id": "formautofill@mozilla.org"
    },
    {
      "name": "Multi-process staged rollout",
      "version": "3.05",
      "id": "e10srollout@mozilla.org"
    },
    {
      "name": "Photon onboarding",
      "version": "1.0",
      "id": "onboarding@mozilla.org"
    },
    {
      "name": "Pocket",
      "version": "1.0.5",
      "id": "firefox@getpocket.com"
    },
    {
      "name": "Shield Recipe Client",
      "version": "76.1",
      "id": "shield-recipe-client@mozilla.org"
    },
    {
      "name": "Web Compat",
      "version": "1.1",
      "id": "webcompat@mozilla.org"
    }
  ]
}
PS I am focusing on attachment 8930451 [details] because the actual live page CPU load seems to vary throughout the day. About half the times I look it has high CPU but at the moment it has low CPU.
(In reply to Mark from comment #16)
> https://perfht.ml/2iGW9bo

This looks like good info.  Thank you!

Hopefully Brian or Hiro can see if anything is going wrong here.

Mark, do you by any chance have have display scaling or the "more space" option enabled in your mac os display preferences?  Another user reported this caused greatly increased CPU usage.

And thank you again for helping us investigate.
Flags: needinfo?(mark.paxman99)
Ben, I was using scaled resolution but no change to the CPU load if I drop to default.
NB only one of these three computers has a retina (high DPI) display, the others work at native. 

I also tried the attachment on my partner's MacBook Air which is running FF56 with E10s disabled by add-ons. High CPU load.

The only pattern I can see across 4 Macs is:-

FF >= 56    high CPU load
FF <= 52    low CPU load

Of course, the Guardian website has been well behaved all today. How annoying :)
Flags: needinfo?(mark.paxman99)
I fired up my Ubuntu VM and saw very much the same result for attachment 8930451 [details]:-

FF >= 56    high CPU load
FF <= 52    low CPU load

Ubuntu 17.04 running in VirtualBox 5.1.30 r118389 (Qt5.6.3) running on MacOS 10.13.1

Using clean installs of the relevant FF versions and top for the CPU loads:-

FF52ESR             ~10%
FF56.0              ~70% + 10%   (E10s enabled)
FF58.0a1 2017-11-11 ~80% + 30%     

Other web pages seem to have sensible CPU loads

I can do you a profiler thingy if you like but I don't see the point, it's all a bit sketchy inside the VM.

PS Happy Thanksgiving!
Thanks Mark for all your help!

Looking at the profiles we're clearly spending a lot of time in display list building, but the mystery to me is why we're doing display list building at all. When I profiled this I made sure the slideshow animation was in view. If it's not in view, then, as hiro points out in comment 14, we'll fail to run it on the compositor but we should still throttle it (due to the optimizations in bug 1166500). Perhaps there's been some difference in the change hints generated meaning we no longer detect that this animation can be throttled? i.e. perhaps KeyframeEffectReadOnly::CanIgnoreIfNotVisible() is returning true?

Can you confirm that the slideshow animation was in view when you took the profile?

The other thing which would be helpful, if you have time, is to try to narrow the regression range somewhat. For example, if you are able to install Firefox 55 (and, ideally 54 and 53 too if necessary) and test that would also help a lot!

(Previous releases are available at: https://ftp.mozilla.org/pub/firefox/releases/)

It's also possible that we never throttled this animation to begin with and something regressed in graphics but, again, having a more narrow regression range would help determine that.
Confirmed.  Keyframes for all animations on the site is;

0% {
    opacity: 0;
}
4% {
    opacity: 1;
}
20% {
    opacity: 1;
}
24% {
    opacity: 0;
}

This keyframes does not have 100% value, so we don't throttle this animations even if the animating element is out of view since we can't calculate cumulative change hint because we don't know the final value (i.e. underlying style for the element) yet when we calculate it.  That's said, as for opacity property, the property produces only Repaintframe, UpdateOpacityLayer and UpdateUsesOpacity, all of them are contained in  nsChangeHint_Hints_CanIgnoreIfNotVisible, so we can throttle it whatever the underlying opacity value is.   We can handle transform property as well.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Brian Birtles (:birtles) from comment #21)
> The other thing which would be helpful, if you have time, is to try to
> narrow the regression range somewhat. For example, if you are able to
> install Firefox 55 (and, ideally 54 and 53 too if necessary) and test that
> would also help a lot!

I think bug 1339332 regressed this, after that bug we use missing keyframe handling for CSS animations as well.
That analysis makes perfect sense. Great work. What's more, it sounds like this is quite easily fixable.

(The fact that a few main thread animations causes such high CPU load is still a concern but hopefully retained display lists help with this.)
Blocks: 1339332
status-firefox57: --- → wontfix
status-firefox58: --- → affected
status-firefox59: --- → affected
Component: General → DOM: Animation
Keywords: regression
Priority: -- → P2
Version: 58 Branch → 55 Branch
Just a quick note (not sure what time zone you are in) & I'll take a proper look over the weekend.

The attachment has two large animated slideshows, one near the top (Mugabe) and one near the bottom (fashion designer). In all my tests I had the top slideshow visible.

You guys are obviously the experts but I just wonder whether there is more going on here than just throttling (sounds like Brian is thinking the same thing). Even if the lower animation was throttled I think the upper animation would be consuming ~40% CPU which is still a lot higher than FF52 & Chrome & Safari). Anyway let's see what you guys come up with.  I am happy to test any fixes.

(I think FF57 uses more CPU in general for animations than FF52, I might do some proper comparisons.)

FF57 on Mac on attachment 8930451 [details]:- I see minor changes in CPU load if the slideshows are out of view
- upper slideshow visible, ~80% (content) + ~40% (FF)
- no slideshows visible   ~70% + 30%
- lower slideshow visible ~80% + 40%
Mark, would you mind trying a binary in this try  (you can get it from target.dmg link) ?  This binary should fix the case in comment 22.  I mean scrolled-out animations on the site should not consume CPU so much with the binary.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40f630240c1b746325d1062414c2aee988518425&selectedJob=147401571

(In reply to Mark from comment #25)
> You guys are obviously the experts but I just wonder whether there is more
> going on here than just throttling (sounds like Brian is thinking the same
> thing). Even if the lower animation was throttled I think the upper
> animation would be consuming ~40% CPU which is still a lot higher than FF52
> & Chrome & Safari). Anyway let's see what you guys come up with.  I am happy
> to test any fixes.

Yeah, I think you are right.  There are probably other things caused these high CPU usage between Firefox 52 and 56.
I tried the binary you posted and it seems much better. I can see the throttling working as I scroll up and down the page

On MacOS 10.13.1

- top animation fully visible      ~25% Nightly  + ~10% Web content
- no animations visible            ~2%  Nightly  + ~5%  Web content   (yes, single digits)
- bottom animation fully visible   ~20% Nightly  + ~10% Web content 

I think there is some other stuff going on on the page which is responsible for the higher-than-ideal 25%+10% CPU, I think it's not just the large slideshows. Hmmmm, I might have a look at that.

If you want me to do a double check you could post a Linux binary and I could try that in Ubuntu.

I would like to look at the live page and check over the next few days, ATM the live page has no slideshows so the issue does not emerge. I will keep looking.

So thanks very much, I think you have fixed it!
If I turn WebRender on with your patched version the slideshows use massive CPU, ~100% Nightly + ~100% Web Content.
Would the WebRender guys be interested in taking a look?
Great!  Thank you, Mark!

(In reply to Mark from comment #27)
> I tried the binary you posted and it seems much better. I can see the
> throttling working as I scroll up and down the page
> 
> On MacOS 10.13.1
> 
> - top animation fully visible      ~25% Nightly  + ~10% Web content
> - no animations visible            ~2%  Nightly  + ~5%  Web content   (yes,
> single digits)
> - bottom animation fully visible   ~20% Nightly  + ~10% Web content 

Nice summary. :)

> If you want me to do a double check you could post a Linux binary and I
> could try that in Ubuntu.

You can get it from the same try (https://treeherder.mozilla.org/#/jobs?repo=try&revision=40f630240c1b746325d1062414c2aee988518425&selectedJob=147403628), target.tar.bz2 for Linux64.

> So thanks very much, I think you have fixed it!

Thank you as well.  I appreciate your helps.  You have been contributing to improve animation performance a lot!

(In reply to Mark from comment #28)
> If I turn WebRender on with your patched version the slideshows use massive
> CPU, ~100% Nightly + ~100% Web Content.
> Would the WebRender guys be interested in taking a look?

Definitely yes.  Could you please open a bug for it?  Also I think it's worth filing the case that the visible animations consume much CPU than  firefox 52.
Note, there is a "best books of 2017" animation on washingtonpost.com at the moment that uses a ton of CPU, even when not visible due to being scrolled off screen.  Would this fix help there as well?

The animation styles from inspector:

url(https://www.washingtonpost.com/r/2010-2019/WashingtonPost/2017/11/15/Interactivity/Images/10books-slider.jpg);

background-repeat: repeat-x;

height: 100%;

background-size: 112%;

animation: bbmovepic 900s linear infinite;

    animation-duration: 900s;
    animation-timing-function: linear;
    animation-delay: 0s;
    animation-direction: normal;
    animation-fill-mode: none;
    animation-iteration-count: infinite;
    animation-play-state: running;
    animation-name: bbmovepic;
Flags: needinfo?(dholbert)
And that wapo animation is gone from the site.  Nevermind.
Flags: needinfo?(dholbert)
(In reply to Mark from comment #28)
> If I turn WebRender on with your patched version the slideshows use massive
> CPU, ~100% Nightly + ~100% Web Content.
> Would the WebRender guys be interested in taking a look?

Bug 1419851 might be related here (although it still shouldn't be 100%).
Fixed on Ubuntu Linux , same story ~30% total CPU with either slideshow visible, <10% CPU with slideshows scrolled off screen. Great!

[  The whole fade transition thing is a can of worms though, watch out more bugs incoming. Take cover! :) ]

Mark
(In reply to Brian Birtles (:birtles) from comment #32)
> (In reply to Mark from comment #28)
> > If I turn WebRender on with your patched version the slideshows use massive
> > CPU, ~100% Nightly + ~100% Web Content.
> > Would the WebRender guys be interested in taking a look?
> 
> Bug 1419851 might be related here (although it still shouldn't be 100%).

It seems that bug 1419851 is the one of the reason, there might be other bugs though since the symptoms happens with the patch.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #22)

> for opacity property, the property produces only Repaintframe,
> UpdateOpacityLayer and UpdateUsesOpacity, all of them are contained in 
> nsChangeHint_Hints_CanIgnoreIfNotVisible, so we can throttle it whatever the
> underlying opacity value is.   We can handle transform property as well.

I missed that transform change might cause nsChangeHint_UpdateOverflow (e.g. transform: none -> others).  So with this way we can't throttle transform animations.  Given that transform property is popular for animations and composite:add and composite:accumulate are not popular as of today, I think we can use base style values for missing keyframes to calculate cumulative change hint in the case composite is 'replace' (i.e. for CSS animations currently).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=de2e61f6cedac551af4a736558938e8306c645f0
Assignee: nobody → hikezoe
I don't know if bug 1406360 is helpful, but the reporter there isolated the issue to a couple of animations.
Comment on attachment 8931976 [details]
Bug 1419079 - Drop checking the pref for animations-api core in file_restyles.html.

https://reviewboard.mozilla.org/r/203030/#review208744
Attachment #8931976 - Flags: review?(bbirtles) → review+
Comment on attachment 8931977 [details]
Bug 1419079 - Drop checking the pref for offscreen throttling.

https://reviewboard.mozilla.org/r/203032/#review208746
Attachment #8931977 - Flags: review?(bbirtles) → review+
Comment on attachment 8931978 [details]
Bug 1419079 - Revise a comment for a test case for missing keyframes.

https://reviewboard.mozilla.org/r/203034/#review208748

::: dom/animation/test/mozilla/file_restyles.html:809
(Diff revision 1)
>  
>        await ensureElementRemoval(div);
>      }
>    );
>  
> -  // Tests that additive animations don't throttle at all.
> +  // Tests that missing keyframes animations don't throttle at all.

(It might be even better as, "Test that animations with missing keyframes aren't throttled.")
Attachment #8931978 - Flags: review?(bbirtles) → review+
Comment on attachment 8931979 [details]
Bug 1419079 - Add a test case for additive animation with a missing keyframe.

https://reviewboard.mozilla.org/r/203036/#review208750
Attachment #8931979 - Flags: review?(bbirtles) → review+
(In reply to Brian Birtles (:birtles) from comment #44)
> Comment on attachment 8931978 [details]
> Bug 1419079 - Revise a comment for a test case for missing keyframes.
> 
> https://reviewboard.mozilla.org/r/203034/#review208748
> 
> ::: dom/animation/test/mozilla/file_restyles.html:809
> (Diff revision 1)
> >  
> >        await ensureElementRemoval(div);
> >      }
> >    );
> >  
> > -  // Tests that additive animations don't throttle at all.
> > +  // Tests that missing keyframes animations don't throttle at all.
> 
> (It might be even better as, "Test that animations with missing keyframes
> aren't throttled.")

Never mind, I see we rewrite this comment in part 5 anyway.
Comment on attachment 8931980 [details]
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'.

https://reviewboard.mozilla.org/r/203038/#review208752

::: dom/animation/KeyframeEffectReadOnly.cpp:1794
(Diff revision 1)
> +    auto getBaseValue =
> +      [&](const KeyframeEffectReadOnly& aKeyframe,
> +          nsCSSPropertyID aProperty) -> const AnimationValue& {
> +        if (basePropertyValue.IsNull()) {
> +          basePropertyValue = aKeyframe.BaseStyle(aProperty);
> +        }
> +        return basePropertyValue;
> +      };

I *think* you can drop the aKeyframe argument and call BaseStyle directly?

From what I can tell, for a capture-default of &, the this pointer is treated specially and captured by value.

Unless we have static analysis that prohibits this or there is some other reason we need to pass this in here?

::: dom/animation/KeyframeEffectReadOnly.cpp:1814
(Diff revision 1)
> +      const AnimationValue& fromValue = segment.mFromValue.IsNull()
> +        ? getBaseValue(*this, property.mProperty)
> +        : segment.mFromValue;

This doesn't seem right. When we don't have a from value, we shouldn't use the base value but instead we should use the underlying value.

So I wonder if we can actually do this optimization at all.
(In reply to Brian Birtles (:birtles) from comment #47)
> ::: dom/animation/KeyframeEffectReadOnly.cpp:1814
> (Diff revision 1)
> > +      const AnimationValue& fromValue = segment.mFromValue.IsNull()
> > +        ? getBaseValue(*this, property.mProperty)
> > +        : segment.mFromValue;
> 
> This doesn't seem right. When we don't have a from value, we shouldn't use
> the base value but instead we should use the underlying value.
> 
> So I wonder if we can actually do this optimization at all.

Thanks for the review!

As Brian told me on IRC, we need to use the underlying style value in missing keyframes for CSS animations as well.  So, I have to figure out other ways to solve this bug properly. :)
For reference there is a test case here: http://jsbin.com/lujupul/edit?html,css,js,output

(Note that this test case appears to use the base value in Chrome release for me, but uses the underlying value in Chrome canary--perhaps I have turned on some experimental Web platform features but I can't figure out which ones right now.)
Comment on attachment 8931980 [details]
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'.

https://reviewboard.mozilla.org/r/203038/#review208760

Clearing review for now as per comment 48.
Attachment #8931980 - Flags: review?(bbirtles)
A feasible way what I can think of was using the base style values for cumulative change hint if there is no other animation on the same element.  IIRC, that is a way what Brian told me on IRC as well.  But this does not work if the base style transform is 'none', and it's most common case I think, it reproduces UpdateContainingBlock, AddOrRemoveTransform and UpdateOverflow change hint.  So in any way we have to handle these change hints somehow.

Given that transform animations on the compositor have been working well, even the transform animation contains 'transform:none', say something like this;

 0% { transform: translateX(100px); }
 50% { transform: none; }
 100% { transform: translateX(-100px); }

So we can throttle those change hints as well for offscreen transform animations?

As for AddOrRemoveTransform, its purpose is to add NS_FRAME_MAY_BE_TRANSFORMED [1].  The NS_FRAME_MAY_BE_TRANSFORMED flag is also added for animation case as well [2], and in the case where the animating element doesn't yet have nsIFrame (i.e. the element is initially appended to the document or reframing happens), we post RequestRestyle(Layer) I believe.  So I think AddOrRemoveTransform change hint can be throttled for offscreen animations.

As for UpdateOverflow, we probably can handle it just like UpdatePostTransformOverflow, i.e. unthrottling the animation periodically to update overflow region.

As for UpdateContainingBlock, I have no idea about the change hint yet.  But again, given that it works on the compositor side, we should throttle this change hint as well for offscreen animations.

[1] https://hg.mozilla.org/mozilla-central/file/38f49346a200/layout/base/RestyleManager.cpp#l1500
[2] https://hg.mozilla.org/mozilla-central/file/38f49346a200/dom/animation/KeyframeEffectReadOnly.cpp#l1881
Blocks: 1218169
As I noted in bug 1218169 comment 65, invisible opacity animations on google search results also don't have 100% keyframe value.   I don't have a clear idea about transform animations yet, but as for opacity animations, I am convinced that we can throttle it even if the opacity animations don't have 0% or 100% keyframe value.
aiui this bug isn't mac-only. If that is incorrect please fix my platform change.
Keywords: perf
OS: Unspecified → All
Brian in Comment 9 said he didn't see it on Windows so I am not sure. I see it on MacOS and on Ubuntu but the latter is a bit sketchy, it's running in a VM. 

Someone with access to all three OSs should check properly.
Blocks: 1430884
(In reply to Hiroyuki Ikezoe (:hiro) from comment #52)
> As I noted in bug 1218169 comment 65, invisible opacity animations on google
> search results also don't have 100% keyframe value.   I don't have a clear
> idea about transform animations yet, but as for opacity animations, I am
> convinced that we can throttle it even if the opacity animations don't have
> 0% or 100% keyframe value.

I am going to fix only opacity animation case here in this bug, and will fix transform case in bug 1430884.
Comment on attachment 8931980 [details]
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'.

https://reviewboard.mozilla.org/r/203038/#review219080

::: dom/animation/KeyframeEffectReadOnly.cpp:1773
(Diff revision 3)
> +        // Skip bailing out in case of opacity. Because for opacity property, we
> +        // don't produce any change hints other than
> +        // nsChangeHint_Hints_CanIgnoreIfNotVisible so that we can throttle it
> +        // whatever underlying opacity value is.
> +        if (property.mProperty == eCSSProperty_opacity) {
> +          continue;
> +        }

If opacity only ever generates change hints that are included in nsChangeHint_Hints_CanIgnoreIfNotVisible can't we hoist this check into the outer loop? i.e. continue as soon as we see property == eCSSProperty_opacity?
Attachment #8931980 - Flags: review?(bbirtles)
Comment on attachment 8931980 [details]
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'.

https://reviewboard.mozilla.org/r/203038/#review219082

(Sorry, didn't mean to clear the review request.)
Attachment #8931980 - Flags: review?(bbirtles)
(In reply to Brian Birtles (:birtles) from comment #63)
> Comment on attachment 8931980 [details]
> Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the
> case of opacity property even if there is a missing keyframe or composite
> operation is not 'replace'.
> 
> https://reviewboard.mozilla.org/r/203038/#review219080
> 
> ::: dom/animation/KeyframeEffectReadOnly.cpp:1773
> (Diff revision 3)
> > +        // Skip bailing out in case of opacity. Because for opacity property, we
> > +        // don't produce any change hints other than
> > +        // nsChangeHint_Hints_CanIgnoreIfNotVisible so that we can throttle it
> > +        // whatever underlying opacity value is.
> > +        if (property.mProperty == eCSSProperty_opacity) {
> > +          continue;
> > +        }
> 
> If opacity only ever generates change hints that are included in
> nsChangeHint_Hints_CanIgnoreIfNotVisible can't we hoist this check into the
> outer loop? i.e. continue as soon as we see property == eCSSProperty_opacity?

Yes, I think we can do it so far since we don't use those opacity specific change hints at all for now.  Whereas change hints transform is used for the condition to unthrottle transform animations every 200ms.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #65)
> Whereas change hints transform is used for the condition to unthrottle transform animations every 200ms.

*change hints for transform*
(In reply to Hiroyuki Ikezoe (:hiro) from comment #65)
> Yes, I think we can do it so far since we don't use those opacity specific
> change hints at all for now.  Whereas change hints transform is used for the
> condition to unthrottle transform animations every 200ms.

Ok, that sounds better. Perhaps I'm missing something but, as it stands, the patch seems a little odd--we don't update mCumulativeChangeHint at all for an opacity segment that has 'composite: add' etc., but we *do* if it is 'composite: replace'. It seems more consistent to not update it ever for opacity segments?

(Obviously when we come to looking at transform we'll need to do something different, however.)
(In reply to Brian Birtles (:birtles) from comment #67)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #65)
> > Yes, I think we can do it so far since we don't use those opacity specific
> > change hints at all for now.  Whereas change hints transform is used for the
> > condition to unthrottle transform animations every 200ms.
> 
> Ok, that sounds better. Perhaps I'm missing something but, as it stands, the
> patch seems a little odd--we don't update mCumulativeChangeHint at all for
> an opacity segment that has 'composite: add' etc., but we *do* if it is
> 'composite: replace'. It seems more consistent to not update it ever for
> opacity segments?

OK, that's fair.  I will move the check in the outer loop.

> (Obviously when we come to looking at transform we'll need to do something
> different, however.)

Yeah, that's the point I am struggling with now, I haven't yet come up with any good solutions for transform, so I have no clear vision how the function will look like.  Then I did put a minimum change there, yeah, it was a bit odd actually. :)
Comment on attachment 8931980 [details]
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'.

https://reviewboard.mozilla.org/r/203038/#review219108

::: dom/animation/KeyframeEffectReadOnly.cpp:1767
(Diff revision 4)
> +    // Skip calculating cumulative change hints in case of opacity. Because for
> +    // opacity property, we don't produce any change hints other than
> +    // nsChangeHint_Hints_CanIgnoreIfNotVisible so that we can throttle it
> +    // whatever underlying opacity value is, whatever change hints are produced
> +    // for opacity value.

Nit: This comment might be a little more natural if we replace the the sentence beginning "Because..." with something like:

"For the opacity property we don't produce any change hints that are not included in nsChangeHint_Hints_CanIgnoreIfNotVisible so we can throttle opacity animations regardless of the change they produce. This optimization is particularly important since it allows us to throttle opacity animations with missing 0%/100% keyframes."
Attachment #8931980 - Flags: review?(bbirtles) → review+
Status: NEW → ASSIGNED
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a91181c84aa
Drop checking the pref for animations-api core in file_restyles.html. r=birtles
https://hg.mozilla.org/integration/autoland/rev/bdec197f0448
Drop checking the pref for offscreen throttling. r=birtles
https://hg.mozilla.org/integration/autoland/rev/ce3c3c6967d1
Revise a comment for a test case for missing keyframes. r=birtles
https://hg.mozilla.org/integration/autoland/rev/49fa09256a65
Add a test case for additive animation with a missing keyframe. r=birtles
https://hg.mozilla.org/integration/autoland/rev/10ea246d416b
Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'. r=birtles
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: