New issue
Advanced search Search tips

Issue 881863 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Improve FrameMetrics Efficiency on Android

Project Member Reported by yiyix@chromium.org, Sep 7

Issue description

After enable FrameMetrics for all platforms, the total CPU running time is increased by 3.6%-19.3% on some android devices. 

example:
https://bugs.chromium.org/p/chromium/issues/detail?id=876709
https://bugs.chromium.org/p/chromium/issues/detail?id=875984

This bug is used to track for the performance improvements on frame metrics. 
 
Cc: chiniforooshan@chromium.org sadrul@chromium.org
Owner: yiyix@chromium.org
Status: Assigned (was: Untriaged)
Cc: tdres...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 17

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

commit 464072dd905fc2c6aecdd56e4eb6fb4d7ee0954f
Author: yiyix <yiyix@chromium.org>
Date: Mon Sep 17 15:55:43 2018

FrameMetrics: Enable Frame Latency Speed and Acceleration behind a flag

The FrameMetrics relies ConvertableToTraceFormat/TracedValue a lot,
so its running cost increased to ~3.9 from ~0.1 after switching to
Perfetto. In this CL, I hide the two low priorities analysis
Frame Latency Speed and Acceleration behind a flag to lower the cost.

After analyzing more data from Frame Metrics, they will be added back as
needed.

Bug: 881863

Change-Id: I959e861df8c08f7fe56dd95f81c932512d148ae3
Reviewed-on: https://chromium-review.googlesource.com/1204593
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591692}
[modify] https://crrev.com/464072dd905fc2c6aecdd56e4eb6fb4d7ee0954f/ui/latency/frame_metrics.cc
[modify] https://crrev.com/464072dd905fc2c6aecdd56e4eb6fb4d7ee0954f/ui/latency/frame_metrics.h
[modify] https://crrev.com/464072dd905fc2c6aecdd56e4eb6fb4d7ee0954f/ui/latency/frame_metrics_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 20

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

commit 33d38175ff521d9b4046418b5601cf52b5e8b804
Author: yiyix <yiyix@chromium.org>
Date: Thu Sep 20 22:05:53 2018

FrameMetrics: Reduce FrameMetrics reporting frequency

Enabling FrameMetrics caused 5-20% regression on rendering.
(ref: https://bugs.chromium.org/p/chromium/issues/detail?id=876709).
The FrameMetrics reports back to Tracing for each CompositorFrame
submission with the following data:
https://docs.google.com/document/d/1P1OKg3s5V2TwE8nTxLay01rn6JmpLgKDY00myyDuINs/edit?usp=sharing

Reporting stats summary for each frame is not very interesting,
So I reduced the frequency of FrameMetrics reporting to each second.

Bug: 881863,  880560 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I96ce7d5bed824aa8d86fa00311136362081df019
Reviewed-on: https://chromium-review.googlesource.com/1226012
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592966}
[modify] https://crrev.com/33d38175ff521d9b4046418b5601cf52b5e8b804/ui/latency/frame_metrics.cc
[modify] https://crrev.com/33d38175ff521d9b4046418b5601cf52b5e8b804/ui/latency/frame_metrics_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 25

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

commit bee2712586ee4f28e7b805a9f9ea22a1d379a89e
Author: yiyix <yiyix@chromium.org>
Date: Tue Sep 25 17:00:41 2018

FrameMetrics: Remove accumulated precision tests

Since we will use Approximate Square Root instead exact square root
in FrameMetrics (cl: 1225105), there is no need to test precision for
square root accumulator. Removing NoAccumulatedPrecisionErrorBasic,
NoAccumulatedPrecisionErrorBig and NoAccumulatedPrecisionErrorBigger.

Bug:881863

Change-Id: Ib4ccf49e6c58f19587aa97964b76df6568be1629
Reviewed-on: https://chromium-review.googlesource.com/1239262
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593975}
[modify] https://crrev.com/bee2712586ee4f28e7b805a9f9ea22a1d379a89e/ui/latency/frame_metrics_test_common.h
[modify] https://crrev.com/bee2712586ee4f28e7b805a9f9ea22a1d379a89e/ui/latency/windowed_analyzer_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 25

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

commit a5d33b00e277364ba0a49c81d5e4c16c83010f17
Author: yiyix <yiyix@chromium.org>
Date: Tue Sep 25 20:15:39 2018

FrameMetrics: Improve SquareRoot calculation Efficiency on Android

Enabling FrameMetrics caused 5-20% regression on rendering.
(ref: https://bugs.chromium.org/p/chromium/issues/detail?id=876709).
One of many reasons is that square root calculation took too long.
In this cl, I implemented Newton's method to approximate the square
root value.

I ran perftest on a Nexus 4 device and got the following result:
average of running square_root using std::sqrt: 1601-01-01 00:00:23.160 UTC
average of running square_root using fastsqrt: 1601-01-01 00:00:10.629 UTC
Conclusion: It's 2.3x faster with newton's approximation instead of
standard library.

I also ran perftests on linux desktop and got 3%-5% improvements.

(perftests ref: https://chromium-review.googlesource.com/c/chromium/src/+/1222676)
Bug: 881863

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I5feebc32053650f338a92a2e1db6f4f67b20f995
Reviewed-on: https://chromium-review.googlesource.com/1225105
Commit-Queue: Yi Xu <yiyix@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594077}
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/frame_metrics.cc
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/frame_metrics.h
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/frame_metrics_test_common.h
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/frame_metrics_unittest.cc
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/stream_analyzer.cc
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/stream_analyzer_unittest.cc
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/windowed_analyzer.cc
[modify] https://crrev.com/a5d33b00e277364ba0a49c81d5e4c16c83010f17/ui/latency/windowed_analyzer_unittest.cc

Sign in to add a comment