Bug 223549

Summary:

Reduce number of heap allocations on the audio thread in AudioSampleDataSource

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

Media

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, eric.carlson, ews-watchlist, ggaren, glenn, hta, jer.noble, peng.liu6, philipj, sam, sergio, tommyw, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

223226

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Description Chris Dumez 2021-03-19 19:53:22 PDT

Reduce number of heap allocation on the audio thread in AudioSampleDataSource:
1   0x135cac840 WTFCrash
2   0x1361ec5c0 JSC::IntlListFormat::initializeListFormat(JSC::JSGlobalObject*, JSC::JSValue, JSC::JSValue)
3   0x135cea27c WTF::fastMalloc(unsigned long)
4   0x115349f10 WebCore::CAAudioStreamDescription::operator new(unsigned long)
5   0x115349e8c std::__1::__unique_if<WebCore::CAAudioStreamDescription>::__unique_single std::__1::make_unique<WebCore::CAAudioStreamDescription, WebCore::CAAudioStreamDescription const&>(WebCore::CAAudioStreamDescription const&)
6   0x11533ace0 decltype(auto) WTF::makeUnique<WebCore::CAAudioStreamDescription, WebCore::CAAudioStreamDescription const&>(WebCore::CAAudioStreamDescription const&)
7   0x11533ac60 WebCore::AudioSampleDataSource::setInputFormat(WebCore::CAAudioStreamDescription const&)
8   0x1180fb670 WebCore::RealtimeOutgoingAudioSourceCocoa::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long)
9   0x11806bad0 WebCore::RealtimeMediaSource::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long)
10  0x113fcbb24 WebCore::MediaStreamAudioSource::consumeAudio(WebCore::AudioBus&, unsigned long)
11  0x115dd59a8 WebCore::MediaStreamAudioDestinationNode::process(unsigned long)
12  0x115d1017c WebCore::AudioNode::processIfNecessary(unsigned long)
13  0x115d75344 WebCore::BaseAudioContext::processAutomaticPullNodes(unsigned long)
14  0x115d0b2c8 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&)
15  0x117ca79e4 WebCore::AudioDestination::callRenderCallback(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&)
16  0x117cabb2c WebCore::AudioDestinationCocoa::AudioDestinationCocoa(WebCore::AudioIOCallback&, unsigned int, float, bool)::$_16::operator()(WebCore::AudioBus*, unsigned long) const
17  0x117caba30 WTF::Detail::CallableWrapper<WebCore::AudioDestinationCocoa::AudioDestinationCocoa(WebCore::AudioIOCallback&, unsigned int, float, bool)::$_16, void, WebCore::AudioBus*, unsigned long>::call(WebCore::AudioBus*, unsigned long)
18  0x117c713b0 WTF::Function<void (WebCore::AudioBus*, unsigned long)>::operator()(WebCore::AudioBus*, unsigned long) const
19  0x117c70eb0 WebCore::MultiChannelResampler::provideInputForChannel(float*, unsigned long, unsigned int)
20  0x117c7a568 decltype(*(std::__1::forward<WebCore::MultiChannelResampler*&>(fp0)).*fp(std::__1::forward<float*>(fp1), std::__1::forward<unsigned long>(fp1), std::__1::forward<unsigned int&>(fp1))) std::__1::__invoke<void (WebCore::MultiChannelResampler::*&)(float*, unsigned long, unsigned int), WebCore::MultiChannelResampler*&, float*, unsigned long, unsigned int&, void>(void (WebCore::MultiChannelResampler::*&)(float*, unsigned long, unsigned int), WebCore::MultiChannelResampler*&, float*&&, unsigned long&&, unsigned int&)
21  0x117c7a460 std::__1::__bind_return<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), std::__1::tuple<WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1>, std::__1::placeholders::__ph<2>, unsigned int>, std::__1::tuple<float*&&, unsigned long&&>, __is_valid_bind_return<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), std::__1::tuple<WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1>, std::__1::placeholders::__ph<2>, unsigned int>, std::__1::tuple<float*&&, unsigned long&&> >::value>::type std::__1::__apply_functor<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), std::__1::tuple<WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1>, std::__1::placeholders::__ph<2>, unsigned int>, 0ul, 1ul, 2ul, 3ul, std::__1::tuple<float*&&, unsigned long&&> >(void (WebCore::MultiChannelResampler::*&)(float*, unsigned long, unsigned int), std::__1::tuple<WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1>, std::__1::placeholders::__ph<2>, unsigned int>&, std::__1::__tuple_indices<0ul, 1ul, 2ul, 3ul>, std::__1::tuple<float*&&, unsigned long&&>&&)
22  0x117c7a3a4 std::__1::__bind_return<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), std::__1::tuple<WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1>, std::__1::placeholders::__ph<2>, unsigned int>, std::__1::tuple<float*&&, unsigned long&&>, __is_valid_bind_return<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), std::__1::tuple<WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1>, std::__1::placeholders::__ph<2>, unsigned int>, std::__1::tuple<float*&&, unsigned long&&> >::value>::type std::__1::__bind<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1> const&, std::__1::placeholders::__ph<2> const&, unsigned int&>::operator()<float*, unsigned long>(float*&&, unsigned long&&)
23  0x117c7a270 WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::MultiChannelResampler::*)(float*, unsigned long, unsigned int), WebCore::MultiChannelResampler*, std::__1::placeholders::__ph<1> const&, std::__1::placeholders::__ph<2> const&, unsigned int&>, void, float*, unsigned long>::call(float*, unsigned long)
24  0x117c9d2dc WTF::Function<void (float*, unsigned long)>::operator()(float*, unsigned long) const
25  0x117c9d218 WebCore::SincResampler::process(float*, unsigned long)
26  0x117c7125c WebCore::MultiChannelResampler::process(WebCore::AudioBus*, unsigned long)
27  0x117ca784c WebCore::AudioDestinationCocoa::renderOnRenderingThead(unsigned long)
28  0x117ca763c WebCore::AudioDestinationCocoa::renderOnRenderingTheadIfPlaying(unsigned long)
29  0x117ca734c WebCore::AudioDestinationCocoa::render(double, unsigned long long, unsigned int, AudioBufferList*)
30  0x1049fc1f0 WebKit::RemoteAudioDestinationProxy::renderQuantum()

Comment 1 Chris Dumez 2021-03-19 19:56:06 PDT

Created attachment 423800 [details]
Patch

Comment 2 Chris Dumez 2021-03-19 19:57:40 PDT

Created attachment 423801 [details]
Patch

Comment 3 Sam Weinig 2021-03-21 10:34:38 PDT

Comment on attachment 423801 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423801&action=review

> Source/WebCore/ChangeLog:10
> +        Reduce number of heap allocations on the audio thread in AudioSampleDataSource
> +        for performance reasons. I got rid of the heap allocations that I could easily
> +        address. Some trickier ones remain.

Seems like it might be worth annotating the ones that remain and adding a comment indicating that no new heap allocations should be added due to the realtime-ness of things.

> Source/WebCore/ChangeLog:23
> +
> +        * platform/audio/cocoa/AudioSampleDataSource.h:
> +        (WebCore::AudioSampleDataSource::inputDescription const):
> +        * platform/audio/cocoa/AudioSampleDataSource.mm:
> +        (WebCore::AudioSampleDataSource::AudioSampleDataSource):
> +        (WebCore::AudioSampleDataSource::~AudioSampleDataSource):
> +        (WebCore::AudioSampleDataSource::setInputFormat):
> +        (WebCore::AudioSampleDataSource::setOutputFormat):
> +        (WebCore::AudioSampleDataSource::pushSamplesInternal):
> +        (WebCore::AudioSampleDataSource::pushSamples):
> +        (WebCore::AudioSampleDataSource::pullSamplesInternal):
> +        (WebCore::AudioSampleDataSource::pullAvalaibleSamplesAsChunks):
> +        (WebCore::AudioSampleDataSource::pullSamples):

Since the changes aren't all super obvious, can you fill in some details here?

Comment 4 Chris Dumez 2021-03-21 12:12:46 PDT

(In reply to Sam Weinig from comment #3)
> Comment on attachment 423801 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423801&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Reduce number of heap allocations on the audio thread in AudioSampleDataSource
> > +        for performance reasons. I got rid of the heap allocations that I could easily
> > +        address. Some trickier ones remain.
> 
> Seems like it might be worth annotating the ones that remain and adding a
> comment indicating that no new heap allocations should be added due to the
> realtime-ness of things.

Well yes, that’s what I am working on. I am fixing the easy ones now and adding exceptions for the remaining ones (see bug this one is marked as a dependency of).

> 
> > Source/WebCore/ChangeLog:23
> > +
> > +        * platform/audio/cocoa/AudioSampleDataSource.h:
> > +        (WebCore::AudioSampleDataSource::inputDescription const):
> > +        * platform/audio/cocoa/AudioSampleDataSource.mm:
> > +        (WebCore::AudioSampleDataSource::AudioSampleDataSource):
> > +        (WebCore::AudioSampleDataSource::~AudioSampleDataSource):
> > +        (WebCore::AudioSampleDataSource::setInputFormat):
> > +        (WebCore::AudioSampleDataSource::setOutputFormat):
> > +        (WebCore::AudioSampleDataSource::pushSamplesInternal):
> > +        (WebCore::AudioSampleDataSource::pushSamples):
> > +        (WebCore::AudioSampleDataSource::pullSamplesInternal):
> > +        (WebCore::AudioSampleDataSource::pullAvalaibleSamplesAsChunks):
> > +        (WebCore::AudioSampleDataSource::pullSamples):
> 
> Since the changes aren't all super obvious, can you fill in some details
> here?

Ok, I will do that. I thought the changes were trivial here, I either started initializing things earlier on the main thread or I switch from uniqueptr to optional to avoid the heap allocation.

Comment 5 Chris Dumez 2021-03-22 08:40:44 PDT

Created attachment 423889 [details]
Patch

Comment 6 Chris Dumez 2021-03-22 15:10:03 PDT

Comment on attachment 423889 [details]
Patch

Clearing flags on attachment: 423889

Committed r274808 (235608@main): <https://commits.webkit.org/235608@main>

Comment 7 Chris Dumez 2021-03-22 15:10:06 PDT

All reviewed patches have been landed.  Closing bug.

Comment 8 Radar WebKit Bug Importer 2021-03-22 15:11:13 PDT

<rdar://problem/75710830>

Comment 9 Darin Adler 2021-03-22 15:11:48 PDT

Comment on attachment 423889 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423889&action=review

> Source/WebCore/platform/audio/cocoa/AudioSampleDataSource.h:72
> +    const CAAudioStreamDescription* inputDescription() const { return m_inputDescription ? &m_inputDescription.value() : nullptr; }

Seems like we could use a generic function that converts an Optional to a pointer or nullptr.