RESOLVED FIXED 142988
[Darwin] Boost the web process QoS level while handling a synchronous IPC message.
https://bugs.webkit.org/show_bug.cgi?id=142988
Summary [Darwin] Boost the web process QoS level while handling a synchronous IPC mes...
Andreas Kling
Reported 2015-03-23 15:08:19 PDT
<rdar://problem/20264346> If the UIProcess is blocking on a synchronous IPC request to the WebProcess, we should boost the WebProcess's main thread for the duration, to ensure that the WebProcess gets appropriately scheduled by the kernel.
Attachments
Patch (1.92 KB, patch)
2015-03-23 15:12 PDT, Andreas Kling
barraclough: review+
Patch (6.35 KB, patch)
2015-03-25 16:33 PDT, Andreas Kling
koivisto: review+
Andreas Kling
Comment 1 2015-03-23 15:12:06 PDT
Andreas Kling
Comment 2 2015-03-25 16:33:17 PDT
Created attachment 249443 [details] Patch Better patch that doesn't wait for the main thread to complete whatever it's doing before setting the boost.
Antti Koivisto
Comment 3 2015-03-26 10:42:34 PDT
Comment on attachment 249443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249443&action=review > Source/WebKit2/Platform/IPC/Connection.cpp:262 > + ASSERT(pthread_main_np()); > + m_mainThread = pthread_self(); Don't we have any existing way to access the main pthread? > Source/WebKit2/Platform/IPC/Connection.h:312 > + pthread_t m_mainThread { 0 }; I think pthread_t is a pointer so nullptr (or just { }) would be more appropriate. > Source/WebKit2/Platform/IPC/MessageDecoder.cpp:46 > MessageDecoder::~MessageDecoder() > { > +#if HAVE(QOS_CLASSES) > + if (m_qosClassOverride) > + pthread_override_qos_class_end_np(m_qosClassOverride); > +#endif > } It seems weird that this is part of the MessageDecoder destructor. I feel this type should be about decoding messages, not juggling thread priorities.
Andreas Kling
Comment 4 2015-03-26 12:19:30 PDT
(In reply to comment #3) > Comment on attachment 249443 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249443&action=review > > > Source/WebKit2/Platform/IPC/Connection.cpp:262 > > + ASSERT(pthread_main_np()); > > + m_mainThread = pthread_self(); > > Don't we have any existing way to access the main pthread? Sort-of. It will lie and give you the web thread on iOS, and I didn't wanna start pulling on that thread. Pun totally intended. > > Source/WebKit2/Platform/IPC/Connection.h:312 > > + pthread_t m_mainThread { 0 }; > > I think pthread_t is a pointer so nullptr (or just { }) would be more > appropriate. k > > Source/WebKit2/Platform/IPC/MessageDecoder.cpp:46 > > MessageDecoder::~MessageDecoder() > > { > > +#if HAVE(QOS_CLASSES) > > + if (m_qosClassOverride) > > + pthread_override_qos_class_end_np(m_qosClassOverride); > > +#endif > > } > > It seems weird that this is part of the MessageDecoder destructor. I feel > this type should be about decoding messages, not juggling thread priorities. It's a bit awkward perhaps, but look at how MessageDecoder also owns importance assertions on OS X.
Anders Carlsson
Comment 5 2015-03-26 12:33:45 PDT
Comment on attachment 249443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249443&action=review > Source/WebKit2/Platform/IPC/Connection.cpp:690 > + pthread_override_t o = pthread_override_qos_class_start_np(m_mainThread, QOS_CLASS_USER_INTERACTIVE, 0); Please call this override instead of just "o". > Source/WebKit2/Platform/IPC/MessageDecoder.h:63 > + void setQOSClassOverride(pthread_override_t o) { m_qosClassOverride = o; } No O.
Andreas Kling
Comment 6 2015-03-26 15:21:26 PDT
Note You need to log in before you can comment on or make changes to this bug.