Bug 223549 - Reduce number of heap allocations on the audio thread in AudioSampleDataSource
Summary: Reduce number of heap allocations on the audio thread in AudioSampleDataSource
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 223226
  Show dependency treegraph
 
Reported: 2021-03-19 19:53 PDT by Chris Dumez
Modified: 2021-03-22 15:11 PDT (History)
14 users (show)

See Also:


Attachments
Patch (8.65 KB, patch)
2021-03-19 19:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (8.65 KB, patch)
2021-03-19 19:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (11.43 KB, patch)
2021-03-22 08:40 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.