Debug nsAutoLocks cause CPU spike

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: John Gaunt (redfive), Assigned: John Gaunt (redfive))

Tracking

({fixed1.8.1.1})

1.8 Branch
x86
Linux
fixed1.8.1.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
( 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.
(Assignee)

Comment 1

11 years ago
Created attachment 242901 [details] [diff] [review]
add NewLock() DestroyLock statics, 1_8 BRANCH
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
(Assignee)

Comment 3

11 years ago
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.
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
(Assignee)

Comment 5

11 years ago
(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.
(Assignee)

Updated

11 years ago
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+
(Assignee)

Comment 7

11 years ago
Checked in 10/26 with brendan's last comment 
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
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+
(Assignee)

Comment 9

11 years ago
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.