- Revision
- 268690
- Author
- [email protected]
- Date
- 2020-10-19 14:46:04 -0700 (Mon, 19 Oct 2020)
Log Message
[GPU Process] RemoteAudioDestination::render() should not need to dispatch to the main thread to do IPC
https://bugs.webkit.org/show_bug.cgi?id=217920
Reviewed by Alex Christensen.
Source/WebKit:
RemoteAudioDestination::render() should not need to dispatch to the main thread to do IPC as this is bad
for performance and IPC::Connection::sendWithAsyncReply() already does locking internally for thread-safety.
The only thing that was preventing us from calling sendWithAsyncReply() from the audio rendering thread was
a threading assertion in CompletionHandler that made sure that the completion handler was always called on
the thread the completion handler was constructed on. To avoid this issue, I added a paramater to the
CompletionHandler constructor allowing the caller to indicate it expects the completion handler to get called
on the main thread. I am now using this new flag in sendWithAsyncReply() and at its call site in
RemoteAudioDestination::render().
No new tests, no Web-facing behavior change.
* GPUProcess/media/RemoteAudioDestinationManager.cpp:
(WebKit::RemoteAudioDestination::render):
* Platform/IPC/Connection.h:
(IPC::Connection::sendWithAsyncReply):
Source/WTF:
Add an optional parameter to the CompletionHandler constructor allowing the caller to indicate it expects
the completion handler to get call on the main thread instead of the construction thread.
* wtf/CompletionHandler.h:
(WTF::CompletionHandler<Out):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (268689 => 268690)
--- trunk/Source/WTF/ChangeLog 2020-10-19 21:35:42 UTC (rev 268689)
+++ trunk/Source/WTF/ChangeLog 2020-10-19 21:46:04 UTC (rev 268690)
@@ -1,3 +1,16 @@
+2020-10-19 Chris Dumez <[email protected]>
+
+ [GPU Process] RemoteAudioDestination::render() should not need to dispatch to the main thread to do IPC
+ https://bugs.webkit.org/show_bug.cgi?id=217920
+
+ Reviewed by Alex Christensen.
+
+ Add an optional parameter to the CompletionHandler constructor allowing the caller to indicate it expects
+ the completion handler to get call on the main thread instead of the construction thread.
+
+ * wtf/CompletionHandler.h:
+ (WTF::CompletionHandler<Out):
+
2020-10-19 Antoine Quint <[email protected]>
Move remaining Web Animations runtime-enabled features to settings
Modified: trunk/Source/WTF/wtf/CompletionHandler.h (268689 => 268690)
--- trunk/Source/WTF/wtf/CompletionHandler.h 2020-10-19 21:35:42 UTC (rev 268689)
+++ trunk/Source/WTF/wtf/CompletionHandler.h 2020-10-19 21:46:04 UTC (rev 268690)
@@ -31,6 +31,7 @@
namespace WTF {
template<typename> class CompletionHandler;
+enum class CompletionHandlerCallThread { MainThread, ConstructionThread };
// Wraps a Function to make sure it is always called once and only once.
template <typename Out, typename... In>
@@ -40,9 +41,13 @@
CompletionHandler() = default;
template<typename CallableType, class = typename std::enable_if<std::is_rvalue_reference<CallableType&&>::value>::type>
- CompletionHandler(CallableType&& callable)
+ CompletionHandler(CallableType&& callable, CompletionHandlerCallThread callThread = CompletionHandlerCallThread::ConstructionThread)
: m_function(WTFMove(callable))
+#if ASSERT_ENABLED
+ , m_shouldBeCalledOnMainThread(callThread == CompletionHandlerCallThread::MainThread || isMainThread())
+#endif
{
+ UNUSED_PARAM(callThread);
}
CompletionHandler(CompletionHandler&&) = default;
@@ -57,7 +62,7 @@
Out operator()(In... in)
{
- ASSERT(m_wasConstructedOnMainThread == isMainThread());
+ ASSERT(m_shouldBeCalledOnMainThread == isMainThread());
ASSERT_WITH_MESSAGE(m_function, "Completion handler should not be called more than once");
return std::exchange(m_function, nullptr)(std::forward<In>(in)...);
}
@@ -65,7 +70,7 @@
private:
Function<Out(In...)> m_function;
#if ASSERT_ENABLED
- bool m_wasConstructedOnMainThread { isMainThread() };
+ bool m_shouldBeCalledOnMainThread;
#endif
};
@@ -157,5 +162,6 @@
} // namespace WTF
using WTF::CompletionHandler;
+using WTF::CompletionHandlerCallThread;
using WTF::CompletionHandlerCallingScope;
using WTF::CompletionHandlerWithFinalizer;
Modified: trunk/Source/WebKit/ChangeLog (268689 => 268690)
--- trunk/Source/WebKit/ChangeLog 2020-10-19 21:35:42 UTC (rev 268689)
+++ trunk/Source/WebKit/ChangeLog 2020-10-19 21:46:04 UTC (rev 268690)
@@ -1,3 +1,26 @@
+2020-10-19 Chris Dumez <[email protected]>
+
+ [GPU Process] RemoteAudioDestination::render() should not need to dispatch to the main thread to do IPC
+ https://bugs.webkit.org/show_bug.cgi?id=217920
+
+ Reviewed by Alex Christensen.
+
+ RemoteAudioDestination::render() should not need to dispatch to the main thread to do IPC as this is bad
+ for performance and IPC::Connection::sendWithAsyncReply() already does locking internally for thread-safety.
+ The only thing that was preventing us from calling sendWithAsyncReply() from the audio rendering thread was
+ a threading assertion in CompletionHandler that made sure that the completion handler was always called on
+ the thread the completion handler was constructed on. To avoid this issue, I added a paramater to the
+ CompletionHandler constructor allowing the caller to indicate it expects the completion handler to get called
+ on the main thread. I am now using this new flag in sendWithAsyncReply() and at its call site in
+ RemoteAudioDestination::render().
+
+ No new tests, no Web-facing behavior change.
+
+ * GPUProcess/media/RemoteAudioDestinationManager.cpp:
+ (WebKit::RemoteAudioDestination::render):
+ * Platform/IPC/Connection.h:
+ (IPC::Connection::sendWithAsyncReply):
+
2020-10-19 Said Abou-Hallawa <[email protected]>
[GPU Process] Clean the DisplayList recording of the NativeImage
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp (268689 => 268690)
--- trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp 2020-10-19 21:35:42 UTC (rev 268689)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp 2020-10-19 21:46:04 UTC (rev 268690)
@@ -141,14 +141,11 @@
if (m_protectThisDuringGracefulShutdown)
return noErr;
- // FIXME: It is unfortunate we have to dispatch to the main thead here. We should be able to IPC straight from the
- // render thread but this is not supported by sendWithAsyncReply().
- callOnMainThread([this, protectedThis = makeRef(*this), sampleTime, hostTime, numberOfFrames, ioData]() mutable {
- m_connection.connection().sendWithAsyncReply(Messages::RemoteAudioDestinationProxy::RequestBuffer(sampleTime, hostTime, numberOfFrames), [this, protectedThis = WTFMove(protectedThis), ioData](auto startFrame, auto numberOfFramesToRender, auto boundsStartFrame, auto boundsEndFrame) mutable {
- m_ringBuffer->setCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
- m_ringBuffer->fetch(ioData, numberOfFramesToRender, startFrame);
- }, m_id.toUInt64());
- });
+ m_connection.connection().sendWithAsyncReply(Messages::RemoteAudioDestinationProxy::RequestBuffer(sampleTime, hostTime, numberOfFrames), CompletionHandler<void(uint64_t, uint64_t, uint64_t, uint64_t)>([this, protectedThis = makeRef(*this), ioData](auto startFrame, auto numberOfFramesToRender, auto boundsStartFrame, auto boundsEndFrame) mutable {
+ ASSERT(isMainThread());
+ m_ringBuffer->setCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
+ m_ringBuffer->fetch(ioData, numberOfFramesToRender, startFrame);
+ }, CompletionHandlerCallThread::MainThread), m_id.toUInt64());
return noErr;
}
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (268689 => 268690)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2020-10-19 21:35:42 UTC (rev 268689)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2020-10-19 21:46:04 UTC (rev 268690)
@@ -514,12 +514,12 @@
auto encoder = makeUnique<Encoder>(T::name(), destinationID);
uint64_t listenerID = nextAsyncReplyHandlerID();
- addAsyncReplyHandler(*this, listenerID, [completionHandler = WTFMove(completionHandler)] (Decoder* decoder) mutable {
+ addAsyncReplyHandler(*this, listenerID, CompletionHandler<void(Decoder*)>([completionHandler = WTFMove(completionHandler)] (Decoder* decoder) mutable {
if (decoder && decoder->isValid())
T::callReply(*decoder, WTFMove(completionHandler));
else
T::cancelReply(WTFMove(completionHandler));
- });
+ }, CompletionHandlerCallThread::MainThread));
encoder->encode(listenerID);
encoder->encode(message.arguments());
sendMessage(WTFMove(encoder), sendOptions);
