IPC messages starve XPCOM timers in child processes

RESOLVED FIXED in mozilla17

Status

()

Core
IPC
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

unspecified
mozilla17
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [LOE:M])

Attachments

(1 attachment, 3 obsolete attachments)

With bug 774988 resolved, repaints on OOP content being bombed with IPC messages (input events) are still starved.  The remaining problems are most likely due to timers being starved; see bug 761933 comment 26.
This probably affects all XPCOM events, not just timers.
Whiteboard: [LOE:M]
Blocks: 784441
Created attachment 654545 [details] [diff] [review]
Run XPCOM timers through chromium event loop

This fixes the perf problems in bug 761933.
Assignee: nobody → jones.chris.g
Summary: IPC messages starve XPCOM timers → IPC messages starve XPCOM timers in child processes
Created attachment 654963 [details] [diff] [review]
First attempt at a real fix

I hoped that sorta-kinda-but-not-really-fair event dispatch would be good enough, but it's not.  This patch is still miles better than what's current in m-c, but it's 14fps slower on homescreen panning than the hacky patch that actually dispatches timers vs. IPC fairly.

Fiddlesticks.
Created attachment 655101 [details] [diff] [review]
Change nsThread to use the chromium queue

Babies are crying everywhere.
Created attachment 655138 [details] [diff] [review]
Ensure that Tasks and XPCOM events are dispatched with the same priority
Attachment #654545 - Attachment is obsolete: true
Attachment #654963 - Attachment is obsolete: true
Attachment #655101 - Attachment is obsolete: true
Attachment #655138 - Flags: review?(bent.mozilla)
Comment on attachment 655138 [details] [diff] [review]
Ensure that Tasks and XPCOM events are dispatched with the same priority

Review of attachment 655138 [details] [diff] [review]:
-----------------------------------------------------------------

Glad this was a simple fix!

::: ipc/glue/MessagePump.cpp
@@ +210,5 @@
>  #endif
> +
> +  // We can get to this point in startup with Tasks in our loop's
> +  // incoming_queue_ or pending_queue_, but without a matching
> +  // DoWorkRunnable().  In MessagePump::Run() above, we sensitivitely

Nit: Let's use a real word instead of "sensitivitely".
Attachment #655138 - Flags: review?(bent.mozilla) → review+
(In reply to ben turner [:bent] from comment #6)
> Comment on attachment 655138 [details] [diff] [review]
> Ensure that Tasks and XPCOM events are dispatched with the same priority
> 
> Review of attachment 655138 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Glad this was a simple fix!
> 
> ::: ipc/glue/MessagePump.cpp
> @@ +210,5 @@
> >  #endif
> > +
> > +  // We can get to this point in startup with Tasks in our loop's
> > +  // incoming_queue_ or pending_queue_, but without a matching
> > +  // DoWorkRunnable().  In MessagePump::Run() above, we sensitivitely
> 
> Nit: Let's use a real word instead of "sensitivitely".

Fixed.  Also s/enqueue/enqueued/ below.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bcda3ce2bc9
https://hg.mozilla.org/integration/mozilla-inbound/rev/941fff75a9e7
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ee8dd0d4bd7
Green on a Try with a post m-c merge run (see the discussion in bug 774988 comment 6 and on).
https://tbpl.mozilla.org/?tree=Try&rev=eabc0f1e0f2e

Re-landed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/779bdf71cde5
Flags: in-testsuite-
Thanks! :)

Comment 12

5 years ago
++ryan
https://hg.mozilla.org/mozilla-central/rev/779bdf71cde5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Duplicate of this bug: 784441
You need to log in before you can comment on or make changes to this bug.