Closed Bug 207469 Opened 22 years ago Closed 22 years ago

optimize timer usage in nsComposerCommandsUpdater::PrimeUpdateTimer()

Categories

(SeaMonkey :: Composer, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: leon.zhang, Assigned: leon.zhang)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

In nsComposerCommandsUpdater::PrimeUpdateTimer(), timer always be deleted and
recreated each time, to enhance perf, we can reInit it and not create each time.
 btw, add/remove timer in sol8 is time-consumed.
take it
Assignee: composer → leon.zhang
Attached patch proposed fix (obsolete) — Splinter Review
Attachment #124428 - Attachment is obsolete: true
Blocks: 188318
Hardware: PC → All
Comment on attachment 124429 [details] [diff] [review]
proposed fix(good format) 

>Index: editor/composer/src/nsComposerCommandsUpdater.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/editor/composer/src/nsComposerCommandsUpdater.cpp,v
>retrieving revision 1.12
>diff -u -r1.12 nsComposerCommandsUpdater.cpp
>--- editor/composer/src/nsComposerCommandsUpdater.cpp	6 Feb 2003 05:34:47 -0000	1.12
>+++ editor/composer/src/nsComposerCommandsUpdater.cpp	29 May 2003 10:17:22 -0000
>@@ -40,7 +40,8 @@
> #include "nsITransactionManager.h"
> 
> nsComposerCommandsUpdater::nsComposerCommandsUpdater()
>-:  mDOMWindow(nsnull)
>+:  mUpdateTimer(nsnull)

mUpdateTimer is an nsCOMPtr member variable, which is automatically initted to
null, so you don't have to init it here.

> nsComposerCommandsUpdater::PrimeUpdateTimer()
> {
>-  nsresult rv = NS_OK;
>-    
>-  if (mUpdateTimer)
>+  if (!mUpdateTimer)
>   {
>-    // i'd love to be able to just call SetDelay on the existing timer, but
>-    // i think i have to tear it down and make a new one.
>-    mUpdateTimer->Cancel();
>-    mUpdateTimer = NULL;      // free it
>+    nsresult rv = NS_OK;
>+    mUpdateTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
>+    if (NS_FAILED(rv)) return rv;
>   }
>-  
>-  mUpdateTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
>-  if (NS_FAILED(rv)) return rv;
> 
>   const PRUint32 kUpdateTimerDelay = 150;
>   return mUpdateTimer->InitWithCallback(NS_STATIC_CAST(nsITimerCallback*, this),

Looks good. Thanks for fixing! Fix the ctor, and sr=sfraser
Attachment #124429 - Flags: superreview+
Comment on attachment 124429 [details] [diff] [review]
proposed fix(good format) 

similar to 204005. r=Henry
Attachment #124429 - Flags: review+
Comment on attachment 124429 [details] [diff] [review]
proposed fix(good format) 

checked in according to comments of sfraser.
thx sfraser,henry
fixed in trunk
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Verified on trunk per last comments.
Status: RESOLVED → VERIFIED
Blocks: 208446
This patch caused the leak described in bug 215163 due to incorrect use of the
timer API.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: