New issue
Advanced search Search tips

Issue 804032 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug

Blocking:
issue 773549



Sign in to add a comment

[remoting host] Perf regression in WebRTC video scheduler

Project Member Reported by lambroslambrou@chromium.org, Jan 19 2018

Issue description

A perf regression was introduced in revision 515673. It can cause encoded frame sizes to become too large, increasing latency. This was seen in manual testing and perf-tests.

See discussion here:
https://chromium-review.googlesource.com/c/chromium/src/+/743027/9/remoting/protocol/webrtc_frame_scheduler_simple.cc#141

From an email thread:

I tried running a perftest with and without that change for frame duration value ( https://chromium-review.googlesource.com/c/chromium/src/+/743027 ). 
Without that CL:
[139282:139282:0117/113144.904083:409492873461:INFO:protocol_perftest.cc(624)] FPS: 4.62803
[139282:139282:0117/113144.904161:409492873535:INFO:protocol_perftest.cc(625)] Average latency: 428.8 ms
[139282:139282:0117/113144.904191:409492873563:INFO:protocol_perftest.cc(627)] Total size: 899130 bytes
[139282:139282:0117/113144.904212:409492873584:INFO:protocol_perftest.cc(628)] Bandwidth utilization: 21.9011%
[139282:139282:0117/113144.904235:409492873607:INFO:protocol_perftest.cc(632)] Network buffer delay (bufferbloat), average: 2 ms,  max:10 ms
[139282:139282:0117/113144.904258:409492873630:INFO:protocol_perftest.cc(637)] Packet drop rate: 0
[139282:139282:0117/113144.904296:409492873669:INFO:protocol_perftest.cc(638)] Average BW estimate: 2101.21 (actual: 8000)

With that CL:
[139043:139043:0117/113030.036884:409418006261:INFO:protocol_perftest.cc(624)] FPS: 0.897252
[139043:139043:0117/113030.036963:409418006340:INFO:protocol_perftest.cc(625)] Average latency: 1352.27 ms
[139043:139043:0117/113030.037012:409418006386:INFO:protocol_perftest.cc(627)] Total size: 729483 bytes
[139043:139043:0117/113030.037038:409418006410:INFO:protocol_perftest.cc(628)] Bandwidth utilization: 16.3633%
[139043:139043:0117/113030.037063:409418006435:INFO:protocol_perftest.cc(632)] Network buffer delay (bufferbloat), average: 1 ms,  max:6 ms
[139043:139043:0117/113030.037086:409418006458:INFO:protocol_perftest.cc(637)] Packet drop rate: 0
[139043:139043:0117/113030.037125:409418006498:INFO:protocol_perftest.cc(638)] Average BW estimate: 1250.25 (actual: 8000)

So it does look like that change was very negative effect on overall performance.

This was LimitedBandwidth/ProtocolPerfTest.ScrollPerformanceWebrtc/2. You can use this command to run it:
  ./out/foo/remoting_perftests --gtest_filter="Limited*Scroll*Webrtc*/2"

 
Blocking: 773549
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 22 2018

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

commit 000b50b04612ee423c76d0bb31b17a1106029955
Author: Lambros Lambrou <lambroslambrou@chromium.org>
Date: Mon Jan 22 07:16:21 2018

[remoting host] Revert change to params_out->duration

This fixes a perf regression introduced by crrev.com/515673, and sets
the encoder's "duration" parameter to a constant value to avoid a
positive-feedback loop.

Bug:  804032 
Change-Id: I2fc184ebed297c1038d49d8bc3dd885522b62c8f
Reviewed-on: https://chromium-review.googlesource.com/877334
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530820}
[modify] https://crrev.com/000b50b04612ee423c76d0bb31b17a1106029955/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/000b50b04612ee423c76d0bb31b17a1106029955/remoting/protocol/webrtc_frame_scheduler_unittest.cc

Labels: -Pri-3 Merge-Request-65 OS-Linux OS-Mac OS-Windows Pri-1
Requesting merge of revision 000b50b04612ee423c76d0bb31b17a1106029955 into M65.

This is a trivial and safe change which fixes a perf regression in Chromoting host process. It has no effect on Chrome browser.

Project Member

Comment 4 by sheriffbot@chromium.org, Jan 23 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by gov...@chromium.org, Jan 25 2018

Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comment #3. Please merge ASAP. Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6688e1a21420a1ef3cce7fd6922ddb4ccbb4bc7c

commit 6688e1a21420a1ef3cce7fd6922ddb4ccbb4bc7c
Author: Lambros Lambrou <lambroslambrou@chromium.org>
Date: Thu Jan 25 00:30:39 2018

[remoting host] Revert change to params_out->duration

This fixes a perf regression introduced by crrev.com/515673, and sets
the encoder's "duration" parameter to a constant value to avoid a
positive-feedback loop.

Bug:  804032 
Change-Id: I2fc184ebed297c1038d49d8bc3dd885522b62c8f
Reviewed-on: https://chromium-review.googlesource.com/877334
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#530820}(cherry picked from commit 000b50b04612ee423c76d0bb31b17a1106029955)
Reviewed-on: https://chromium-review.googlesource.com/885023
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#81}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/6688e1a21420a1ef3cce7fd6922ddb4ccbb4bc7c/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/6688e1a21420a1ef3cce7fd6922ddb4ccbb4bc7c/remoting/protocol/webrtc_frame_scheduler_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment