Bug 46834 – Limit number of AudioNode deletions per render quantum in AudioContext

• Component: New Bugs
• Status: RESOLVED FIXED

[Chris Rogers]

Limit number of AudioNode deletions per render quantum in AudioContext

[Chris Rogers]

Created attachment 69242 [details]
Patch

[Chris Rogers]

This fixes a problem where garbage collection suddenly releases tons of AudioNodes at the same time and many are deleted in the real-time audio thread.  This patch amortizes the deletions over several render quantums by deleting only a limited amount each time.

NOTE: this patch also contains an incidental minor change to the HRTFDatabaseLoader API.

[Kenneth Russell]

Comment on attachment 69242 [details]
Patch

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

There are several changes in this patch unrelated to this bug that need to be removed.

> WebCore/webaudio/AudioContext.cpp:110
> +    m_hrtfDatabaseLoader = HRTFDatabaseLoader::createAndLoadAsynchronouslyIfNecessary(sampleRate());

This is an unrelated change.

> WebCore/webaudio/AudioContext.cpp:351
> +        m_graphOwnerThread = thisThread;

This also looks like an unrelated change.

> WebCore/webaudio/AudioContext.cpp:438
> +    ASSERT(isAudioThread() && isGraphOwner());

Is this new assertion related to this change?

> WebCore/webaudio/AudioContext.h:248
> +    RefPtr<HRTFDatabaseLoader> m_hrtfDatabaseLoader;

Unrelated changes shouldn't be in this patch.

[Simon Fraser (smfr)]

>  many are deleted in the real-time audio thread

Isn't the right solution to ensure that these are not deleted on that thread, rather than implementing throttling?

[Kenneth Russell]

(In reply to comment #4)
> >  many are deleted in the real-time audio thread
> 
> Isn't the right solution to ensure that these are not deleted on that thread, rather than implementing throttling?

Moving the deletion of these objects to the main thread requires rethinking some of the synchronization and queueing strategy. This is an incremental fix improving the behavior without completely rewriting things.

[Chris Rogers]

(In reply to comment #4)
> >  many are deleted in the real-time audio thread
> 
> Isn't the right solution to ensure that these are not deleted on that thread, rather than implementing throttling?

Sometimes, they have to be deleted in the real-time audio thread because the garbage collection happens before the "note" has finished playing.  Later on, in the real-time thread the note finishes and there's no more chance to delete it in the main thread.  We could implement a callOnMainThread() type of thing, but I think this would introduce performance problems into the main thread.  I think we need the throttling (for the audio thread) but also try to delete the nodes at every opportunity we have in the main thread (such as in the connect(), disconnect(), and deref() methods.

[Chris Rogers]

Created attachment 69244 [details]
Patch

[Kenneth Russell]

Comment on attachment 69244 [details]
Patch

Looks fine to me.

[Chris Rogers]

Created attachment 69252 [details]
Patch

[Chris Rogers]

last minute tweak: changed int to unsigned.

[Kenneth Russell]

Comment on attachment 69252 [details]
Patch

OK.

[Chris Rogers]

Comment on attachment 69252 [details]
Patch

Clearing flags on attachment: 69252

Committed r68689: <http://trac.webkit.org/changeset/68689>

[Chris Rogers]

All reviewed patches have been landed.  Closing bug.