Closed Bug 123988 Opened 23 years ago Closed 23 years ago

nsRecylingAllocator shouldbe install timer if there are no allocations to be reclaimed

Categories

(Core :: XPCOM, defect, P2)

All
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla0.9.9

People

(Reporter: dp, Assigned: dp)

Details

Attachments

(1 file, 2 obsolete files)

nsRecyclingAllocator installs a timer to free memory in its pool if it is was not touched. Would be good to remove the timer once all memory has been released so the apps doesn't wake up every 10 secs just to figure out nothing needs to be done.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.9
dougt r= ? Simon/Brendan sr= ? The idea is to install the timer on the first allocation and remove the timer when all allocations have been freed.
Keywords: nsbeta1
+ (void) NS_NewTimer(&mRecycleTimer, nsRecycleTimerCallback, this, + NS_SEC_TO_MS(mRecycleAfter), NS_PRIORITY_LOWEST, + NS_TYPE_REPEATING_SLACK); Remind me again why mRecycleTimer is not an nsCOMPtr? + void EnableTimer() + void DisableTimer() I'd wouldn't be surprised if the compiler refused to inline these, and the overhead of the methods they call seems high enough that inlining doesn't make too much sense. Their impls might be slightly better in the .cp file. Otherwise, sr=sfraser
No particular reason why mRecycleTimer is not a COMPtr. It could well be. Ok. I inlined the methods by hand. Thanks simon.
Attached patch with suggestions from simon (obsolete) — Splinter Review
inlines the methods by hand.
Attachment #68509 - Attachment is obsolete: true
Attachment #68513 - Attachment is obsolete: true
Comment on attachment 68515 [details] [diff] [review] Removed an incorrect ! Ooops, missed that. One reason why explicit comparisons are a good idea. I'd prefer to see + if (obj->mNAllocations == 0 && obj->mRecycleTimer != nsnull) rather than + if (!obj->mNAllocations && obj->mRecycleTimer) but that's certainly a nit. sr=sfraser
Attachment #68515 - Flags: superreview+
Simon, actually you didnt miss it. The first patch was right. The second one that hand inlined the methods had the issue and I corrected it right away. Thanks.
Comment on attachment 68515 [details] [diff] [review] Removed an incorrect ! r=dougt.
Attachment #68515 - Flags: review+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Component: XP Miscellany → XPCOM
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: