Last Comment Bug 357426 - Debug nsAutoLocks cause CPU spike
: Debug nsAutoLocks cause CPU spike
Status: RESOLVED FIXED
: fixed1.8.1.1
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: 1.8 Branch
: x86 Linux
: -- normal (vote)
: ---
Assigned To: John Gaunt (redfive)
:
: Nathan Froyd [:froydnj]
Mentors:
Depends on:
Blocks: songbird
  Show dependency treegraph
 
Reported: 2006-10-20 12:45 PDT by John Gaunt (redfive)
Modified: 2007-01-16 12:45 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add NewLock() DestroyLock statics, 1_8 BRANCH (2.83 KB, patch)
2006-10-20 12:51 PDT, John Gaunt (redfive)
no flags Details | Diff | Splinter Review
moved NewLock/DLock out of debug section and added another static (3.47 KB, patch)
2006-10-20 19:29 PDT, John Gaunt (redfive)
brendan: review+
jst: superreview+
dveditz: approval1.8.1.1+
Details | Diff | Splinter Review

Description John Gaunt (redfive) 2006-10-20 12:45:52 PDT
( 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.
Comment 1 John Gaunt (redfive) 2006-10-20 12:51:51 PDT
Created attachment 242901 [details] [diff] [review]
add NewLock() DestroyLock statics, 1_8 BRANCH
Comment 2 Brendan Eich [:brendan] 2006-10-20 13:49:43 PDT
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
Comment 3 John Gaunt (redfive) 2006-10-20 19:29:42 PDT
Created attachment 242949 [details] [diff] [review]
moved NewLock/DLock out of debug section and added another static

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.
Comment 4 Brendan Eich [:brendan] 2006-10-20 20:18:08 PDT
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
Comment 5 John Gaunt (redfive) 2006-10-23 12:11:36 PDT
(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.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-24 17:44:36 PDT
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.
Comment 7 John Gaunt (redfive) 2006-10-27 11:32:05 PDT
Checked in 10/26 with brendan's last comment 
Comment 8 Daniel Veditz [:dveditz] 2006-11-29 11:27:30 PST
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
Comment 9 John Gaunt (redfive) 2006-11-30 11:16:59 PST
Checked in 1.8 branch 11/30
Comment 10 Brendan Eich [:brendan] 2007-01-16 12:45:21 PST
Comment on attachment 242949 [details] [diff] [review]
moved NewLock/DLock out of debug section and added another static

Thougth I stamped this; sorry.

/be

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