Possible regression due to img ddk 1.8.

Matteo - can you please use chrome://tracing to take a trace during when this is happening and post the results here.

drinkcat@ - can you try regression testing this w/ ToT and img-ddk 1.7?

I don't think this is a regresion, I tried with trybot-elm-release/R62-9800.0.0-b13402 (ddk 1.7 on ToT), and I still see some stuttering in the falling leaves.

The effect is subtle though, so I'm not sure if I'm looking at the right thing.

drinkcat@ - what FPS do you observe? The stuttering I am talking about only appears when the FPS meter is visible. The FPS I observe are oscillating "chaotically" around 54 FPS. This is not what we expect when running this workload. I understand we expect to hit ~60 FPS when running the falling leaves on elm (Daniel, please correct me if this is not the case).

This 54 FPS behaviour can be observed when running the current "Official Build" (downloaded from Google) dev image (dev-channel elm). I confirmed that the same issue affects a test build I put together on Monday this week. I cannot exclude this was fixed yesterday or in the last 48 hours. I can say, however, that the issue does not affect beta-channel elm (see above to have info about the exact versions I am referring to). By "not affect" I mean that when I tick on the "FPS meter" checkbox I may see some initial stuttering, but the FPS soon stabilises around 60 FPS and only occasionally deviates from this value.

The issue occurs both when running on battery and when running on AC.

Daniel - I took some traces yesterday. I can anticipate that differences between the traces with FPS meter and without are pretty macroscopic. I was trying to use telemetry to re-take them (to remove the user-factor, i.e. myself moving the mouse and boosting the CPU freq), but telemetry seems to be able to magically hide the FPS meter when replaying the page (despite I put the option in /etc/chrome_dev.conf and passed --show-fps-counter to run_benchmark). Anyway, one way or another I should be able to post some traces soon.

#4: I only tested ToT (~9800), with ddk 1.8 (current) and 1.7 (revert).

I saw that the FPS is mostly 60, but with a number of spikes to lower values. Also, the leaves themselves do not fall as smoothly when the FPS meter is active. I did not compare with earlier release (R60), as you did, so maybe it's less obvious to me.

I might try to bisect tomorrow if I have some free time, I'll let you know.

drinkcat@ - Thanks for looking into this.

Daniel - I attach two traces. They were both obtained on the same elm device, using a similar procedure (I couldn't convince telemetry to do this for me, the FPS meter disappears as soon as the Page.navigate SendRequest is handled). I took some care of touching the device as little as possible. In particular, I recommend examining the central region of the traces between time = 3s ... 9s. Let me know if you need more info.

Deleted: trace_without_fps_meter.json.gz 4.3 MB

trace_without_fps_meter.json.gz 4.3 MB Download

Deleted: trace_with_fps_meter.json.gz 4.7 MB

trace_with_fps_meter.json.gz 4.7 MB Download

Matteo: You're right, now that I've seen the behaviour on older versions, the difference is pretty obvious: Flat 60fps on older builds, very spiky on newer ones (average at ~54fps)

Also, I managed to bisect to:
 - Last good: R61-9632.0.0
 - First bad: R61-9633.0.0

https://crosland.corp.google.com/log/9632.0.0..9633.0.0 (I don't see anything of interest there)
https://chromium.googlesource.com/chromium/src/+log/61.0.3124.0..61.0.3125.0?pretty=fuller&n=10000

If anything, this CL looks very suspicious:
https://codereview.chromium.org/2752833002
""
commit	bdc4a31b6e88e8a12b0b7bb298a03b020b1c61de	
author	sohan.jyoti <sohan.jyoti@huawei.com>	Thu Jun 08 01:09:51 2017

cc: Use gpu raster in HUD.

This changes the SW back-end for drawing HUD to use Ganesh
backed canvas and texture for drawing FPS, raster and gpu
memory info. If gpu is not available it falls back to old
SW canvas implementation.

BUG= 345416 
""

ericrk/danakj/sohanjg: Any idea?

drinkcat@ - Great! Thanks for the bisect. I reverted the patch you mentioned together with 5f3480305411ab8bd094a2f7f82245c4997a10b9 "cc: HUD handle context lost in Gpu raster." (just because the two were entangled) and I confirm this fixes the issue. I did this on my local build (test image) for elm.

Hmm, a superficial glance at the CL suggests this might be a perf hit from doing CPU accesses directly on an uncached bo.  This would suggest we should see a similar regression on other ARM devices that use WC bos (kevin, veyron), but not, for example, on intel.

drinkcat@ if you have time, can you do a quick test on an x86 device to see if it regresses or not?

dianders@ if you have time, can you have someone do a quick check on kevin or similar device?

Hey...I had a look, the HUD indeed is causing a frame drop in this scenario.
I will put up a patch to temporarily use SW raster again for HUD, till we fix it.
Sorry for the trouble.

Patch here, https://chromium-review.googlesource.com/c/602349

@9: I'm not sure it really reproduces on kevin.  I tried both of the builds above:

* R61-9632.0.0
* R61-9633.0.0

On both of them kevin mostly sits at 60 FPS with the occasional glitch down to 59 or so.  I can't seem to tell any real difference...

Daniel, dianders - I also didn't find any noticeable difference on kevin (as in my bug description above). I wonder whether a difference can be spotted by looking into the traces, though.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5eb08fbc487c137d4b6072adf0b4c74d87647b9f

commit 5eb08fbc487c137d4b6072adf0b4c74d87647b9f
Author: sohan <sohan.jyoti@huawei.com>
Date: Fri Aug 04 20:41:20 2017

cc: Temporarily avoid gpu raster drawing in HUD.

Drawing HUD with gpu raster is causing some frame drops and
stuttering. Fix that before re-eanbling.

BUG= 751732 

Change-Id: Ia62cfe8c1aa6e16bdd4737e9c8bb2de5b4b66fca
Reviewed-on: https://chromium-review.googlesource.com/602349
Commit-Queue: Sohan Jyoti Ghosh <sohan.jyoti@huawei.com>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Reviewed-by: Sohan Jyoti Ghosh <sohan.jyoti@huawei.com>
Cr-Commit-Position: refs/heads/master@{#492117}
[modify] https://crrev.com/5eb08fbc487c137d4b6072adf0b4c74d87647b9f/cc/trees/layer_tree_host_impl.cc

#10: Thanks Sohan for the quick fix.

#8: Thanks for checking Matteo. If you have time, could you also confirm that the patch in #14 fixes the issue as well?

Sohan, thanks for putting together a fix in such a short time :-)
Sohan, drinkcat I am OOO right now, but I should be able to try the patch later today.

drinkcat, I cherry picked the patch in #14 (5eb08fb) on my Chromium tree (which is a bit behind, at 43f4d76) and I confirm that it does indeed fix the issue. 

I was having some cycles to check the issue again for root cause, I re-checked with enabling gpu-rastered HUD on ToT.
The frame drops and stuttering with the css animation doesn't seem to be there anymore, at least on Android and Linux.

Matteo@, can you please re-check by reverting patch at #14, on ToT, if the issue is still visible in your configuration.
Thanks.

Sohan, sorry for the late reply (I was out of office on holiday). I should be able to carry out a check in the next 24h.

Sohan, I synced yesterday (to 27e688a) and reverted 5eb08fb, as you suggested. I couldn't find any substantial difference between the measurements given by the FPS Hud with and without the revert. In particular, I measured (by hand) the number of dropped frames in the two cases for a duration of 120s and I got 21 (without revert) and 22 (with the revert). I only did one measurement but it is definitely clear that something changed in the codebase and fixed the issue. Sohan, do you have an idea of what this might be?

I additionally repeated the experiments done at https://bugs.chromium.org/p/chromium/issues/detail?id=748603, where I measured the CPU frequencies while running the leaves workload (https://webkit.org/blog-files/leaves/) with different cgroup settings on Elm. I confirm that:

1. Reverting 5eb08fb doesn't cause any change in the frequencies chosen by the Elm (interactive) governor.
2. In general, having the HUD shown on the screen causes non-negligible changes in the frequencies chosen by the CPU governor. In other words, the FPS HUD has an non-negligible impact on the system behaviour and on what it is trying to measure, the FPS. This is true for Elm but - to a lesser degree - also for Kevin.

Thanks for verifying Matteo. I will prepare a revert for the patch, to re-enable Gpu rastered HUD.
Regarding the reason how it got solved, since its primarily about toggling skia gpu rasterization, something with the system config (gpu) and/or skia maybe playing a role. But i am not entirely sure.

@22: There are known bugs on imagination with gpu rasterization, see https://bugs.chromium.org/p/chromium/issues/detail?id=763517 about the things that need fixing before we can enable it.

@23, I see, that looks to be the problem indeed, the assumption was context provider would not be created for devices w/o using gpu, but its better to check for gpu raster status explicitly for os and gpu configuration.

Updated new patch with it https://chromium-review.googlesource.com/662758 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/519cc42a59ddc6220512f91253b718ef4912afe5

commit 519cc42a59ddc6220512f91253b718ef4912afe5
Author: sohan <sohan.jyoti@huawei.com>
Date: Thu Sep 14 20:22:37 2017

cc: Check gpu raster status for hud.

This inserts the gpu raster status check before using it
for drawing hud canvas. For some gpu and os configurations
we should still use sw hud canvas instead.

BUG= 751732 

Change-Id: Ibe4a8161142d9632fcde0ac81a6cce772a8fea6e
Reviewed-on: https://chromium-review.googlesource.com/662758
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Commit-Queue: Sohan Jyoti Ghosh <sohan.jyoti@huawei.com>
Cr-Commit-Position: refs/heads/master@{#502032}
[modify] https://crrev.com/519cc42a59ddc6220512f91253b718ef4912afe5/cc/trees/layer_tree_host_impl.cc