Closed Bug 357426 Opened 14 years ago Closed 14 years ago

Debug nsAutoLocks cause CPU spike

Categories

(Core :: XPCOM, defect)

1.8 Branch
x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mozilla, Assigned: mozilla)

References

Details

(Keywords: fixed1.8.1.1)

Attachments

(1 file, 1 obsolete file)

( copied w/edit from bug 58904 )
We see in Songbird, in debug mode a major thrashing of the OrderTable. We currently use nsAutoLocks in our db code and make a TON of queries. The symptom we see is CPU spiking ( 50% on a 2 CPU machine meaning one cpu is always pegged ) and then the app becomes very un-responsive. Of course in release this symptom goes away.

I have been able to make the symptom go away a couple of ways, 1) change
nsAutoLock back to PRLock/PRUnlock (duh) 2) hack nsAutoLock to have static
methods as does nsAutoMonitor - NewLock() and DestroyLock() that operate
identically to the nsAutoMonitor calls.

I have a patch for 2) that I'll be attaching.
Attachment #242901 - Flags: review?(brendan)
Comment on attachment 242901 [details] [diff] [review]
add NewLock() DestroyLock statics, 1_8 BRANCH

Since you're here, how about static functions to consolidate the #ifdef DEBUG code in NewLock and NewMonitor?  Thanks,

/be
Had to move the NewLock/DestroyLock methods out of the big ifdef DEBUG section (whoops). Added the method requested.

Patch applies cleanly to trunk and branch.
Assignee: nobody → mozilla
Attachment #242901 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #242949 - Flags: review?(brendan)
Attachment #242901 - Flags: review?(brendan)
Comment on attachment 242949 [details] [diff] [review]
moved NewLock/DLock out of debug section and added another static

Move the OrderTable null test into OnSemaphoreRecycle and r=me.

/be
(In reply to comment #4)
> (From update of attachment 242949 [details] [diff] [review] [edit])
> Move the OrderTable null test into OnSemaphoreRecycle and r=me.
> 
> /be
> 

Oh yeah, duh. Needs to be there. I was trying to save the call when there wasn't an OrderTable, but that really doesn't make any sense.
Attachment #242949 - Flags: superreview?(jst)
Comment on attachment 242949 [details] [diff] [review]
moved NewLock/DLock out of debug section and added another static

sr=jst with brendan's issue fixed before checking in.
Attachment #242949 - Flags: superreview?(jst) → superreview+
Checked in 10/26 with brendan's last comment 
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #242949 - Flags: approval1.8.1.1?
Comment on attachment 242949 [details] [diff] [review]
moved NewLock/DLock out of debug section and added another static

approved for 1.8 branch, a=dveditz for drivers
Attachment #242949 - Flags: approval1.8.1.1? → approval1.8.1.1+
Checked in 1.8 branch 11/30
Keywords: fixed1.8.1.1
Comment on attachment 242949 [details] [diff] [review]
moved NewLock/DLock out of debug section and added another static

Thougth I stamped this; sorry.

/be
Attachment #242949 - Flags: review?(brendan) → review+
You need to log in before you can comment on or make changes to this bug.