Issue metadata
Sign in to add a comment
|
1.8% regression in webrtc_perf_tests at 18542:18542 |
||||||||||||||||||||
Issue descriptionneteq_performance/10_pl_10_drift
,
Jul 3 2017
The range points to https://chromium.googlesource.com/external/webrtc/+/0703856b53e80159d666e289762406989f08ccfa Not sure if it makes sense though. kwiberg@ PTAL
,
Jul 3 2017
The next build is worse, associated with https://chromium.googlesource.com/external/webrtc/+/36b1a5fcec6270ec4a5bea87b33a49b418a3cb29
,
Jul 6 2017
Investigating. This is probably caused by frames being muted and then unmuted, causing a memset to 0. Prior to my change, the caller may have skipped the zeroing if it knew it was going to overwrite the buffer anyway. Given the age of the change, is it reasonable to fix forward rather than rolling back?
,
Jul 6 2017
Depending on how easy is a fix and how risky is a rollback. If nothing is built on top of this CL and if it does not contain important fixes itself, then it is reasonable to roll back. Regression in Issue 738839 is quite big.
,
Jul 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/3ffa72d0f03b75812c0ce367c1958ff9a80a6aeb commit 3ffa72d0f03b75812c0ce367c1958ff9a80a6aeb Author: Jonathan Yu <yujo@chromium.org> Date: Sat Jul 08 23:36:26 2017 Add AudioFrame::ResetWithoutMuting() to address performance regression. Prior to https://codereview.webrtc.org/2750783004/ Reset() intentionally did not zero out the buffer. After that change, callers calling Reset() and then mutable_data() were performing a wasteful zeroing. This change adds ResetWithoutMuting() to match the old behavior and switches the sole non-test caller of Reset() to use ResetWithoutMuting() instead. Prior to this change (optimized, Linux): $ out/Default/webrtc_perf_tests --gtest_filter=NetEqPerformanceTest.Run* \ --gtest_repeat=10 | grep neteq_performance *RESULT neteq_performance: 10_pl_10_drift= 4051 ms *RESULT neteq_performance: 0_pl_0_drift= 1768 ms *RESULT neteq_performance: 10_pl_10_drift= 3666 ms *RESULT neteq_performance: 0_pl_0_drift= 1690 ms *RESULT neteq_performance: 10_pl_10_drift= 3685 ms *RESULT neteq_performance: 0_pl_0_drift= 1693 ms *RESULT neteq_performance: 10_pl_10_drift= 3720 ms *RESULT neteq_performance: 0_pl_0_drift= 1690 ms *RESULT neteq_performance: 10_pl_10_drift= 3780 ms *RESULT neteq_performance: 0_pl_0_drift= 1728 ms *RESULT neteq_performance: 10_pl_10_drift= 3733 ms *RESULT neteq_performance: 0_pl_0_drift= 1737 ms *RESULT neteq_performance: 10_pl_10_drift= 3781 ms *RESULT neteq_performance: 0_pl_0_drift= 1744 ms *RESULT neteq_performance: 10_pl_10_drift= 3712 ms *RESULT neteq_performance: 0_pl_0_drift= 1731 ms *RESULT neteq_performance: 10_pl_10_drift= 3681 ms *RESULT neteq_performance: 0_pl_0_drift= 1691 ms *RESULT neteq_performance: 10_pl_10_drift= 3681 ms *RESULT neteq_performance: 0_pl_0_drift= 1690 ms With this change: $ out/Default/webrtc_perf_tests --gtest_filter=NetEqPerformanceTest.Run* \ --gtest_repeat=10 | grep neteq_performance *RESULT neteq_performance: 10_pl_10_drift= 3824 ms *RESULT neteq_performance: 0_pl_0_drift= 1632 ms *RESULT neteq_performance: 10_pl_10_drift= 3502 ms *RESULT neteq_performance: 0_pl_0_drift= 1521 ms *RESULT neteq_performance: 10_pl_10_drift= 3520 ms *RESULT neteq_performance: 0_pl_0_drift= 1534 ms *RESULT neteq_performance: 10_pl_10_drift= 3517 ms *RESULT neteq_performance: 0_pl_0_drift= 1530 ms *RESULT neteq_performance: 10_pl_10_drift= 3521 ms *RESULT neteq_performance: 0_pl_0_drift= 1527 ms *RESULT neteq_performance: 10_pl_10_drift= 3511 ms *RESULT neteq_performance: 0_pl_0_drift= 1533 ms *RESULT neteq_performance: 10_pl_10_drift= 3518 ms *RESULT neteq_performance: 0_pl_0_drift= 1523 ms *RESULT neteq_performance: 10_pl_10_drift= 3503 ms *RESULT neteq_performance: 0_pl_0_drift= 1524 ms *RESULT neteq_performance: 10_pl_10_drift= 3514 ms *RESULT neteq_performance: 0_pl_0_drift= 1534 ms *RESULT neteq_performance: 10_pl_10_drift= 3501 ms *RESULT neteq_performance: 0_pl_0_drift= 1530 ms BUG= webrtc:7343 , chromium:738852 , chromium:738839 Change-Id: Idcbb276ca0ed27fff95164a73f1c1fa310175ee5 Reviewed-on: https://chromium-review.googlesource.com/563021 Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org> Reviewed-by: Olga Sharonova <olka@webrtc.org> Reviewed-by: Tommi <tommi@webrtc.org> Commit-Queue: Tommi <tommi@webrtc.org> Cr-Commit-Position: refs/heads/master@{#18939} [modify] https://crrev.com/3ffa72d0f03b75812c0ce367c1958ff9a80a6aeb/webrtc/modules/audio_coding/neteq/sync_buffer.cc [modify] https://crrev.com/3ffa72d0f03b75812c0ce367c1958ff9a80a6aeb/webrtc/modules/include/module_common_types.h
,
Jul 11 2017
Graphs are back down to normal, marking fixed. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by olka@chromium.org
, Jul 3 2017