Closed Bug 1311834 Opened 4 years ago Closed 4 years ago

MSCOM MainThreadInvoker: Spin the background thread while waiting for main thread

Categories

(Core :: IPC, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: aklotz, Assigned: aklotz)

References

(Blocks 1 open bug)

Details

(Whiteboard: aes+)

Attachments

(1 file)

Performance measurements show that the biggest source of perf regression between MTA and STA communication is the cost of blocking the RPC background thread while waiting on the main thread to execute.

As-is, it is possible for the main thread to be blocked (ie, it is in MsgWaitForMultipleObjectsEx) and then the RPC thread blocks waiting for the main thread to wake up and process the APC. Modifying the main thread to poll for messages and APCs instead of blocking fixed the problem. OTOH, we can't have our content process main thread burning up CPU like that. :-)

OTOH, the RPC background thread only waits for a short period of time for the main thread to run its calls. Spinning that thread for such a short time wouldn't be too big a deal.

Subsequent performance tests demonstrate that this brings out-of-process enumeration of all visible accessibles in a gmail tab down to par with non-e10s or better.

Of course, doing this on a uniprocessor box doesn't give us anything, so we'll need to fall back to blocking in that case.
Whiteboard: aes+
Note that with this patch, the entire impetus for bug 1304891 goes away.
Comment on attachment 8803118 [details]
Bug 1311834: Make MainThreadInvoker use a spin loop on multiprocessor machines;

https://reviewboard.mozilla.org/r/87348/#review86596

::: ipc/mscom/MainThreadInvoker.cpp:29
(Diff revision 1)
> +#define CPU_PAUSE() _mm_pause()
> +#elif defined(__GNUC__) || defined(__clang__)
> +#define CPU_PAUSE() __builtin_ia32_pause()
> +#endif
> +
> +static bool sIsMulticore = false;

nit - no need for the false assignment here.

::: ipc/mscom/MainThreadInvoker.cpp:123
(Diff revision 1)
>    PRUint32 tid = ::PR_GetThreadID(mainPrThread);
>    sMainThread = ::OpenThread(SYNCHRONIZE | THREAD_SET_CONTEXT, FALSE, tid);
> +
> +  SYSTEM_INFO sysInfo;
> +  ::GetSystemInfo(&sysInfo);
> +  sIsMulticore = sysInfo.dwNumberOfProcessors > 1;

This is available through NS_SYSTEMINFO_CONTRACTID. Safe to use that here instead?
Comment on attachment 8803118 [details]
Bug 1311834: Make MainThreadInvoker use a spin loop on multiprocessor machines;

https://reviewboard.mozilla.org/r/87348/#review86604
Attachment #8803118 - Flags: review?(jmathies) → review+
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9839d6c2a187
Make MainThreadInvoker use a spin loop on multiprocessor machines; r=jimm
https://hg.mozilla.org/mozilla-central/rev/9839d6c2a187
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.