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

Issue 734490 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 758695

Blocking:
issue 735861



Sign in to add a comment

Reenable realtime priority audio threads on 2-core Chrome OS machines

Project Member Reported by maxmorin@chromium.org, Jun 19 2017

Issue description

After https://chromium-review.googlesource.com/c/535560/, 2-core Chrome OS machines will no longer use real-time audio threads on the renderer-side. This is due to issue 710245. To allow usage of real-time threads, one of the following alternative fixes to 710245 must be implemented (these are the ones I come up with right now):
1. Rewrite the blink allocator to not use spinlocks, or make those spinlocks somehow support priority inheritance. Probably only on Chrome OS. This might have bad impact on performance in general, not just for audio?
2. Rewrite the TaskQueue to not use a blink Deque.
3. Don't use PostTask in AudioDeviceThread callbacks.
4. Taking it further: never take locks/allocate memory in AudioDeviceThread callbacks. This may be ideal, but is of course very difficult.
 
Components: Internals>Media>Audio

Comment 2 by olka@google.com, Jun 21 2017

Cc: olka@chromium.org

Comment 3 by olka@google.com, Jun 21 2017

Cc: alexclarke@chromium.org palmer@chromium.org
Maybe the WTF::Deque in the TaskQueue can use another allocator? I see it takes an allocator as a template parameter. I'm not sure if this would have performance implications for the rest of the code though.

Comment 5 by palmer@google.com, Jun 21 2017

That might be a good way to use an allocator that pre-allocates, which is the only way to really get real-time performance. So that approach SGTM.
Currently the task queue is WTF::Deque<Task, 8> which means it only performs dynamic allocations if more than 8 entries are in it. When I checked a few weeks ago the audio threads never seemed to go above 3 entries.  Of course the Task's themselves involve memory allocations probably from the malloc heap, I don't know if that's a problem or not. 
Blocking: 735861
Blockedon: 758695
Should be fine to reenable realtime priority after https://chromium-review.googlesource.com/c/chromium/src/+/873647. Might want some QA help to make really really sure though. Does the Chrome OS audio team want to take this up?
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 12 2018

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

commit 42a128b7ad14524b01bcf4da74b4d0aa836b3c61
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Feb 12 13:24:48 2018

Reenable realtime audio threads on 2-core Chrome OS.

Workaround shouldn't be needed anymore after crrev.com/530835.
Reenabling realtime priority should reduce audio glitches.

This really shouldn't cause any regressions, but adding the old
bugs as FYI just in case.

Bug:  754213 , 734490 ,710245, 770312 ,803419
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I2f7c6c793fade47a205a896a25468a7312ba55de
Reviewed-on: https://chromium-review.googlesource.com/913272
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536069}
[modify] https://crrev.com/42a128b7ad14524b01bcf4da74b4d0aa836b3c61/media/audio/audio_device_thread.cc

Cc: -maxmorin@chromium.org
Owner: maxmorin@chromium.org
Status: Fixed (was: Available)

Sign in to add a comment