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

Issue 590998 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

41.2%-610.8% regression in webrtc_perf_tests at 11713:11713

Project Member Reported by minyue@chromium.org, Mar 1 2016

Issue description

This must be related to the following change. Please verify that the degradation is intended.

https://chromium.googlesource.com/external/webrtc/+/18fcbcf48
c190b6248cd16ef044d1edb79cba040
 
Owner: aluebs@chromium.org
I would expect this patch to be more complex than before, but 600% is a little bit too much. I will look into it and report back.
Cc: peah@chromium.org
It seems to me that the increase in complexity for mac is reasonable, since the VAD started running on the render thread. But in Windows it seems to be too much and it also leaks to the capture thread complexity, which points towards a threading problem in the IE (which we are already aware of). What is your view on it, Per? I will continue looking into it, but I suspect this extreme regression should disappear once we fix the threading model of the IE.
Please disregard my blaming of the threading model in #4. The actual issue is in the vectorisation of the fmaf function. A fix was uploaded here: https://codereview.webrtc.org/1755943002/
Components: Blink>WebRTC>Audio
Labels: -M-48 -Pri-2 M-50 Pri-1 OS-Mac OS-Windows
aluebs@ - once https://codereview.webrtc.org/1755943002/ lands and the affected perf graphs look good, please let us know if you want that fix to be considered for an M50 merge. The WebRTC 50 branch was cut at r11765 (aka https://codereview.webrtc.org/1739713002), so it currently includes this regression. 
Cc: tnakamura@chromium.org
No, I don't think a merge makes sense, since this component isn't enabled anywhere.
Labels: -Pri-1 -M-50 M-51 Pri-2
Moving to M51 and downgrading to P2 per #7. Thanks for the clarification!
Status: Fixed (was: Assigned)
I am marking this as fixed, since the CL has landed and the performance bots returned to reasonable levels. Please note that they are not back to the original levels, since the CL that introduced this regression added a VAD to the render side, so a increase in complexity is expected. But at least there are no 600% regressions anymore.

Sign in to add a comment