784647 - IPC messages starve XPCOM timers in child processes

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This probably affects all XPCOM events, not just timers.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Run XPCOM timers through chromium event loop

This fixes the perf problems in bug 761933.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Change nsThread to use the chromium queue

Babies are crying everywhere.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Ensure that Tasks and XPCOM events are dispatched with the same priority

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2bcda3ce2bc9

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/941fff75a9e7
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ee8dd0d4bd7

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Thanks! :)

Comment 12

[Andreas Gal :gal]

++ryan

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/779bdf71cde5