Title: [274672] trunk/Source/WebCore
- Revision
- 274672
- Author
- [email protected]
- Date
- 2021-03-18 13:58:23 -0700 (Thu, 18 Mar 2021)
Log Message
Avoid heap allocation under BaseAudioContext::sourceNodeDidFinishPlayback()
https://bugs.webkit.org/show_bug.cgi?id=223461
Reviewed by Jer Noble.
BaseAudioContext::sourceNodeDidFinishPlayback() was appending to a m_finishedSourceNodes
Vector, which would cause heap allocations on the audio thread. To avoid such heap
allocations (which are bad for performance), we now set a "isFinishedSourceNode" flag
on the AudioNode. When BaseAudioContext::derefFinishedSourceNodes(), it relies on
this new isFinishedSourceNode flag instead of using the m_finishedSourceNodes Vector.
* Modules/webaudio/AudioNode.h:
(WebCore::AudioNode::isFinishedSourceNode const):
(WebCore::AudioNode::setIsFinishedSourceNode):
* Modules/webaudio/BaseAudioContext.cpp:
(WebCore::BaseAudioContext::~BaseAudioContext):
(WebCore::BaseAudioContext::uninitialize):
(WebCore::isFinishedSourceNode):
(WebCore::BaseAudioContext::derefFinishedSourceNodes):
(WebCore::BaseAudioContext::sourceNodeDidFinishPlayback):
* Modules/webaudio/BaseAudioContext.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (274671 => 274672)
--- trunk/Source/WebCore/ChangeLog 2021-03-18 20:42:10 UTC (rev 274671)
+++ trunk/Source/WebCore/ChangeLog 2021-03-18 20:58:23 UTC (rev 274672)
@@ -1,3 +1,27 @@
+2021-03-18 Chris Dumez <[email protected]>
+
+ Avoid heap allocation under BaseAudioContext::sourceNodeDidFinishPlayback()
+ https://bugs.webkit.org/show_bug.cgi?id=223461
+
+ Reviewed by Jer Noble.
+
+ BaseAudioContext::sourceNodeDidFinishPlayback() was appending to a m_finishedSourceNodes
+ Vector, which would cause heap allocations on the audio thread. To avoid such heap
+ allocations (which are bad for performance), we now set a "isFinishedSourceNode" flag
+ on the AudioNode. When BaseAudioContext::derefFinishedSourceNodes(), it relies on
+ this new isFinishedSourceNode flag instead of using the m_finishedSourceNodes Vector.
+
+ * Modules/webaudio/AudioNode.h:
+ (WebCore::AudioNode::isFinishedSourceNode const):
+ (WebCore::AudioNode::setIsFinishedSourceNode):
+ * Modules/webaudio/BaseAudioContext.cpp:
+ (WebCore::BaseAudioContext::~BaseAudioContext):
+ (WebCore::BaseAudioContext::uninitialize):
+ (WebCore::isFinishedSourceNode):
+ (WebCore::BaseAudioContext::derefFinishedSourceNodes):
+ (WebCore::BaseAudioContext::sourceNodeDidFinishPlayback):
+ * Modules/webaudio/BaseAudioContext.h:
+
2021-03-18 Devin Rousso <[email protected]>
CSP headers can block default <video> controls
Modified: trunk/Source/WebCore/Modules/webaudio/AudioNode.h (274671 => 274672)
--- trunk/Source/WebCore/Modules/webaudio/AudioNode.h 2021-03-18 20:42:10 UTC (rev 274671)
+++ trunk/Source/WebCore/Modules/webaudio/AudioNode.h 2021-03-18 20:58:23 UTC (rev 274672)
@@ -191,6 +191,9 @@
ChannelInterpretation channelInterpretation() const { return m_channelInterpretation; }
virtual ExceptionOr<void> setChannelInterpretation(ChannelInterpretation);
+ bool isFinishedSourceNode() const { return m_isFinishedSourceNode; }
+ void setIsFinishedSourceNode() { m_isFinishedSourceNode = true; }
+
protected:
// Inputs and outputs must be created before the AudioNode is initialized.
void addInput();
@@ -248,6 +251,7 @@
bool m_isMarkedForDeletion { false };
bool m_isDisabled { false };
+ bool m_isFinishedSourceNode { false };
#if DEBUG_AUDIONODE_REFERENCES
static bool s_isNodeCountInitialized;
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp (274671 => 274672)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2021-03-18 20:42:10 UTC (rev 274671)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2021-03-18 20:58:23 UTC (rev 274672)
@@ -171,7 +171,6 @@
ASSERT(m_isStopScheduled);
ASSERT(m_nodesToDelete.isEmpty());
ASSERT(m_referencedSourceNodes.isEmpty());
- ASSERT(m_finishedSourceNodes.isEmpty());
ASSERT(m_automaticPullNodes.isEmpty());
if (m_automaticPullNodesNeedUpdating)
m_renderingAutomaticPullNodes.resize(m_automaticPullNodes.size());
@@ -244,7 +243,7 @@
AutoLocker locker(*this);
// This should have been called from handlePostRenderTasks() at the end of rendering.
// However, in case of lock contention, the tryLock() call could have failed in handlePostRenderTasks(),
- // leaving nodes in m_finishedSourceNodes. Now that the audio thread is gone, make sure we deref those nodes
+ // leaving nodes in m_referencedSourceNodes. Now that the audio thread is gone, make sure we deref those nodes
// before the BaseAudioContext gets destroyed.
derefFinishedSourceNodes();
}
@@ -575,14 +574,21 @@
return IIRFilterNode::create(scriptExecutionContext, *this, WTFMove(options));
}
+static bool isFinishedSourceNode(const AudioConnectionRefPtr<AudioNode>& node)
+{
+ return node->isFinishedSourceNode();
+}
+
void BaseAudioContext::derefFinishedSourceNodes()
{
ASSERT(isGraphOwner());
ASSERT(isAudioThread() || isAudioThreadFinished());
- for (auto& node : m_finishedSourceNodes)
- derefSourceNode(*node);
- m_finishedSourceNodes.clear();
+ if (!m_hasFinishedAudioSourceNodes)
+ return;
+
+ m_referencedSourceNodes.removeAllMatching(isFinishedSourceNode);
+ m_hasFinishedAudioSourceNodes = false;
}
void BaseAudioContext::refSourceNode(AudioNode& node)
@@ -1055,7 +1061,8 @@
{
ASSERT(isAudioThread());
- m_finishedSourceNodes.append(&node);
+ node.setIsFinishedSourceNode();
+ m_hasFinishedAudioSourceNodes = true;
}
void BaseAudioContext::workletIsReady()
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h (274671 => 274672)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2021-03-18 20:42:10 UTC (rev 274671)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2021-03-18 20:58:23 UTC (rev 274672)
@@ -319,9 +319,9 @@
void workletIsReady();
// When source nodes begin playing, the BaseAudioContext keeps them alive inside m_referencedSourceNodes.
- // When the nodes stop playing, they get added to m_finishedSourceNodes. After each rendering quantum,
- // we call derefSourceNode() on every node in m_finishedSourceNodes since we no longer need to keep them
- // alive.
+ // When the nodes stop playing, a flag gets set on the AudioNode accordingly. After each rendering quantum,
+ // we call derefFinishedSourceNodes() to remove those nodes from m_referencedSourceNodes since we no longer
+ // need to keep them alive.
void refSourceNode(AudioNode&);
void derefSourceNode(AudioNode&);
@@ -350,9 +350,6 @@
Ref<AudioWorklet> m_worklet;
- // Only accessed in the audio thread.
- Vector<AudioNode*> m_finishedSourceNodes;
-
// Either accessed when the graph lock is held, or on the main thread when the audio thread has finished.
Vector<AudioConnectionRefPtr<AudioNode>> m_referencedSourceNodes;
@@ -412,6 +409,7 @@
AudioIOPosition m_outputPosition;
HashMap<String, Vector<AudioParamDescriptor>> m_parameterDescriptorMap;
+ bool m_hasFinishedAudioSourceNodes { false };
// These are cached per audio context for performance reasons. They cannot be
// static because they rely on the sample rate.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
