1370323 - BackgroundHangMonitor spends a lot of time spinning on its CondVar

Status: NEW

(Core :: XPCOM, enhancement, P3)

Description

[(Away)]

Noticed in bug 1359550 comment 22 and I could also see it in local profiles on Windows. The BHM code is the heaviest user of SleepConditionVariableSRW.

From reading disassembly, it seems SleepConditionVariableSRW will spin up to 1024 times with a "pause" instruction before it sleeps the thread. This doesn't appear to be controllable on a per-CV basis.

I wonder if there's some other synchronization primitive that would do the job just as well here? If not, I'm not sure this is worth doing too much contortion for.

Comment 1

[Eric Rahm [:erahm]]

A couple thoughts:

#1 - We could backout bug 1364624 (I'm not convinced we saw a big win from it, no talos bumps at all)
#2 - We could just do a windows-only ifdef using critical sections and SleapConiditonVariableCS for BHM
#3 - Maybe some of the quantum flow work that decreased the hang reporter time is making this worse and the SRW is actually no worse than the CS

Comment 2

[Eric Rahm [:erahm]]

For #3 I was thinking of bug 1346415 which I thought dropped the report threshold, but maybe it didn't?

Comment 3

[(Away)]

SleepConditionVariableCS has a very similar spin loop, so I don't think it would be any better.

Comment 4

[Eric Rahm [:erahm]]

Any chance we could try reverting bug 1346415 locally to see if that helps?

We also might be able to use |WaitForSingleObject| with a |Semaphore| instead or possibly a |WaitableTimer| (I don't fully recall the semantics of that though).