Last Comment Bug 784647 - IPC messages starve XPCOM timers in child processes
: IPC messages starve XPCOM timers in child processes
Status: RESOLVED FIXED
[LOE:M]
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
Mentors:
: 784441 (view as bug list)
Depends on:
Blocks: 761933 784441
  Show dependency treegraph
 
Reported: 2012-08-22 03:50 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-09-13 18:08 PDT (History)
9 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Run XPCOM timers through chromium event loop (4.02 KB, patch)
2012-08-23 01:20 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
First attempt at a real fix (16.39 KB, patch)
2012-08-24 02:35 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
Change nsThread to use the chromium queue (10.08 KB, patch)
2012-08-24 12:41 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
Ensure that Tasks and XPCOM events are dispatched with the same priority (2.21 KB, patch)
2012-08-24 13:48 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
bent.mozilla: review+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-22 03:50:47 PDT
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 2012-08-22 03:51:13 PDT
This probably affects all XPCOM events, not just timers.
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-23 01:20:10 PDT
Created attachment 654545 [details] [diff] [review]
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 2012-08-24 02:35:19 PDT
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.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-24 12:41:36 PDT
Created attachment 655101 [details] [diff] [review]
Change nsThread to use the chromium queue

Babies are crying everywhere.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-24 13:48:00 PDT
Created attachment 655138 [details] [diff] [review]
Ensure that Tasks and XPCOM events are dispatched with the same priority
Comment 6 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-24 14:23:47 PDT
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 2012-08-24 17:46:43 PDT
(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 2012-08-25 01:26:31 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bcda3ce2bc9
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-08-25 12:55:02 PDT
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 2012-08-25 12:57:48 PDT
Thanks! :)
Comment 12 Andreas Gal :gal 2012-08-25 13:02:01 PDT
++ryan
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-08-25 19:26:26 PDT
https://hg.mozilla.org/mozilla-central/rev/779bdf71cde5
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-13 18:08:34 PDT
*** Bug 784441 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.