Bug 142988 – [Darwin] Boost the web process QoS level while handling a synchronous IPC message.

• Component: Platform
• Status: RESOLVED FIXED

[Andreas Kling]

<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.

[Andreas Kling]

Created attachment 249282 [details]
Patch

[Andreas Kling]

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 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]

(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 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]

Committed r182028: <http://trac.webkit.org/changeset/182028>