New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 816727 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

chromoting host consumes too much CPU resources when using webrtc

Project Member Reported by sergeyu@chromium.org, Feb 27 2018

Issue description

1. Connect to chromoting host using webrtc-based client.
2. Observe that the host constantly consumes more than a one CPU core, even when the screen is static. In addition to that Xvfb consumes about 30%:

Top 3 threads as reported by top:
%CPU %MEM     TIME+ COMMAND                                                                               
63.7  0.3   0:19.43 ChromotingCaptu                                                                                                   
35.1  0.2   6:52.46 Xvfb                                                                                                              
33.5  0.3   0:12.81 ChromotingNetwo 

I see this issue with 65.0.3325.39 and on trunk builds of the host. Numbers above from the trunk build. On 65* CPU consumption is higher because always sends 5fps before https://chromium-review.googlesource.com/c/chromium/src/+/907591. 
 
Cc: lambroslambrou@chromium.org
It looks like the bug is caused by this change: https://chromium-review.googlesource.com/c/chromium/src/+/738933 
After that CL when screen is static the host tries to capture the screen at a rate that's bound only by the CPU performance while previously it was was guaranteed to be below 30 fps. Note that PacingBucket.GetEmptyTime() may return time in the past.
I suggest to revert that CL - I doubt that it provides any improvement that cannot be achieved by simply increasing kTargetFrameRate.
Cc: -lambroslambrou@chromium.org zijiehe@chromium.org
Owner: lambroslambrou@chromium.org
Status: Started (was: Untriaged)
Eek! WebrtcFrameSchedulerSimple::CaptureNextFrame() is getting called in a tight loop! That CL was part of Zijie's work to support H264 encode. It was a while ago - I don't know if it makes sense to revert that, or if it will revert cleanly. I think I'd rather just fix the logic to ensure we don't capture more often than kTargetFrameRate.

Also, if we need to spin a new M65 release, we should aim for a simple fix.

Comment 4 by zijiehe@google.com, Feb 27 2018

Looking...
I have a simple fix, just writing a unittest for this now.

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 1 2018

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

commit 65f6394de68d8e96184234f5b7c8bf1e80f699f1
Author: Lambros Lambrou <lambroslambrou@chromium.org>
Date: Thu Mar 01 00:36:53 2018

[remoting host] Rate-limit capturer to 30FPS

For WebRTC-enabled connections, ensure capturer is not scheduled more
often than 30FPS, to limit CPU usage.

Bug:  816727 
Change-Id: If2efd9de0749848f209fad7ed1926be108f402c3
Reviewed-on: https://chromium-review.googlesource.com/940449
Reviewed-by: Zijie He <zijiehe@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539957}
[modify] https://crrev.com/65f6394de68d8e96184234f5b7c8bf1e80f699f1/remoting/protocol/webrtc_frame_scheduler_simple.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 1 2018

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

commit 66afc5e5d10127546cc4b98b9117aff588b5e66b
Author: Lambros Lambrou <lambroslambrou@chromium.org>
Date: Thu Mar 01 20:48:46 2018

[remoting host] Simplify capture scheduling logic

This removes an unnecessary section from
WebrtcFrameSchedulerSimple::ScheduleNextFrame()
to address review comments on
https://chromium-review.googlesource.com/940449
and potentially improves performance and b/w utilization.

Bug:  816727 
Change-Id: I76aaf326a48f985880ea7ff4ba4e16cc246862b3
Reviewed-on: https://chromium-review.googlesource.com/942484
Reviewed-by: Zijie He <zijiehe@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540276}
[modify] https://crrev.com/66afc5e5d10127546cc4b98b9117aff588b5e66b/remoting/protocol/webrtc_frame_scheduler_simple.cc

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 13 2018

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

commit f018c9b26cd58e0438c6f28bfcaa8245fdb69fa8
Author: Lambros Lambrou <lambroslambrou@chromium.org>
Date: Tue Mar 13 00:24:24 2018

[remoting host] Add unittest for capturing at 30FPS.

This adds a unittest for the bug fixed in
http://crrev.com/65f6394de68d8e96184234f5b7c8bf1e80f699f1

This updates the scheduler and unittests to use a mock clock
implementation from TestMockTimeTaskRunner, instead of just
mocking the current time directly.

Bug:  816727 
Change-Id: I194af8e65bae70f0af3a202e0bbb7653059abcaf
Reviewed-on: https://chromium-review.googlesource.com/947575
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542666}
[modify] https://crrev.com/f018c9b26cd58e0438c6f28bfcaa8245fdb69fa8/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/f018c9b26cd58e0438c6f28bfcaa8245fdb69fa8/remoting/protocol/webrtc_frame_scheduler_simple.h
[modify] https://crrev.com/f018c9b26cd58e0438c6f28bfcaa8245fdb69fa8/remoting/protocol/webrtc_frame_scheduler_unittest.cc

Sign in to add a comment