Fixing bug 366393. Attempt to schedule GC between pageloads when possible rather than slowing down page load with GC calls while loading. r+sr=bugmail@sicking.cc

dom/src/base/nsJSEnvironment.cpp

@@ -140,8 +140,19 @@ static PRLogModuleInfo* gJSDiagnostics;
 #endif
 #endif // WINCE
 
-#define NS_GC_DELAY                2000 // ms
-#define NS_FIRST_GC_DELAY          10000 // ms
+// The amount of time we wait between a request to GC (due to leaving
+// a page) and doing the actual GC.
+#define NS_GC_DELAY                 2000 // ms
+
+// The amount of time we wait until we force a GC in case the previous
+// GC timer happened to fire while we were in the middle of loading a
+// page (we'll GC once the page is loaded if that happens before this
+// amount of time has passed).
+#define NS_LOAD_IN_PROCESS_GC_DELAY 4000 // ms
+
+// The amount of time we wait from the first request to GC to actually
+// doing the first GC.
+#define NS_FIRST_GC_DELAY           10000 // ms
 
 #define JAVASCRIPT nsIProgrammingLanguage::JAVASCRIPT
 
@@ -150,6 +161,19 @@ static PRLogModuleInfo* gJSDiagnostics;
 static nsITimer *sGCTimer;
 static PRBool sReadyForGC;
 
+// The number of currently pending document loads. This count isn't
+// guaranteed to always reflect reality and can't easily as we don't
+// have an easy place to know when a load ends or is interrupted in
+// all cases. This counter also gets reset if we end up GC'ing while
+// we're waiting for a slow page to load. IOW, this count may be 0
+// even when there are pending loads.
+static PRUint32 sPendingLoadCount;
+
+// Boolean that tells us whether or not the current GC timer
+// (sGCTimer) was scheduled due to a GC timer firing while we were in
+// the middle of loading a page.
+static PRBool sLoadInProgressGCTimer;
+
 nsScriptNameSpaceManager *gNameSpaceManager;
 
 static nsIJSRuntimeService *sRuntimeService;
@@ -3001,7 +3025,7 @@ nsJSContext::FinalizeContext()
 void
 nsJSContext::GC()
 {
-  FireGCTimer();
+  FireGCTimer(PR_FALSE);
 }
 
 void
@@ -3124,18 +3148,62 @@ nsJSContext::Notify(nsITimer *timer)
 {
   NS_ASSERTION(mContext, "No context in nsJSContext::Notify()!");
 
-  // nsCycleCollector_collect() will run a ::JS_GC indirectly,
-  // so we do not explicitly call ::JS_GC here. 
-  nsCycleCollector_collect();
+  NS_RELEASE(sGCTimer);
+
+  if (sPendingLoadCount == 0 || sLoadInProgressGCTimer) {
+    // nsCycleCollector_collect() will run a ::JS_GC() indirectly,
+    // so we do not explicitly call ::JS_GC() here. 
+    nsCycleCollector_collect();
+
+    sLoadInProgressGCTimer = PR_FALSE;
+
+    // Reset sPendingLoadCount in case the timer that fired was a
+    // timer we scheduled due to a normal GC timer firing while
+    // documents were loading. If this happens we're waiting for a
+    // document that is taking a long time to load, and we effectively
+    // ignore the fact that the currently loading documents are still
+    // loading and move on as if they weren't.
+    sPendingLoadCount = 0;
+  } else {
+    FireGCTimer(PR_TRUE);
+  }
 
   sReadyForGC = PR_TRUE;
 
-  NS_RELEASE(sGCTimer);
   return NS_OK;
 }
 
+// static
 void
-nsJSContext::FireGCTimer()
+nsJSContext::LoadStart()
+{
+  ++sPendingLoadCount;
+}
+
+// static
+void
+nsJSContext::LoadEnd()
+{
+  // sPendingLoadCount is not a well managed load counter (and doesn't
+  // need to be), so make sure we don't make it wrap backwards here.
+  if (sPendingLoadCount > 0) {
+    --sPendingLoadCount;
+  }
+
+  if (!sPendingLoadCount && sLoadInProgressGCTimer) {
+    sGCTimer->Cancel();
+
+    NS_RELEASE(sGCTimer);
+    sLoadInProgressGCTimer = PR_FALSE;
+
+    // nsCycleCollector_collect() will run a ::JS_GC() indirectly, so
+    // we do not explicitly call ::JS_GC() here.
+    nsCycleCollector_collect();
+  }
+}
+
+void
+nsJSContext::FireGCTimer(PRBool aLoadInProgress)
 {
   // Always clear the newborn roots.  If there's already a timer, this
   // will let the GC from that timer clean up properly.  If we're going
@@ -3153,7 +3221,9 @@ nsJSContext::FireGCTimer()
   if (!sGCTimer) {
     NS_WARNING("Failed to create timer");
 
-    ::JS_GC(mContext);
+    // nsCycleCollector_collect() will run a ::JS_GC() indirectly, so
+    // we do not explicitly call ::JS_GC() here.
+    nsCycleCollector_collect();
 
     return;
   }
@@ -3161,9 +3231,13 @@ nsJSContext::FireGCTimer()
   static PRBool first = PR_TRUE;
 
   sGCTimer->InitWithCallback(this,
-                             first ? NS_FIRST_GC_DELAY : NS_GC_DELAY,
+                             first ? NS_FIRST_GC_DELAY :
+                             aLoadInProgress ? NS_LOAD_IN_PROCESS_GC_DELAY :
+                                               NS_GC_DELAY,
                              nsITimer::TYPE_ONE_SHOT);
 
+  sLoadInProgressGCTimer = aLoadInProgress;
+
   first = PR_FALSE;
 }
 
@@ -3259,6 +3333,8 @@ nsJSRuntime::Startup()
   // initialize all our statics, so that we can restart XPCOM
   sGCTimer = nsnull;
   sReadyForGC = PR_FALSE;
+  sLoadInProgressGCTimer = PR_FALSE;
+  sPendingLoadCount = 0;
   gNameSpaceManager = nsnull;
   sRuntimeService = nsnull;
   sRuntime = nsnull;

dom/src/base/nsJSEnvironment.h

@@ -42,10 +42,10 @@
 #include "nsCOMPtr.h"
 #include "jsapi.h"
 #include "nsIObserver.h"
-#include "nsIScriptSecurityManager.h"
 #include "nsIXPCScriptNotify.h"
 #include "nsITimer.h"
 #include "prtime.h"
+
 class nsIXPConnectJSObjectHolder;
 
 class nsJSContext : public nsIScriptContext,
@@ -167,6 +167,9 @@ public:
 
   NS_DECL_NSITIMERCALLBACK
 
+  static void LoadStart();
+  static void LoadEnd();
+
 protected:
   nsresult InitializeExternalClasses();
   nsresult InitializeLiveConnectClasses(JSObject *aGlobalObj);
@@ -181,7 +184,7 @@ protected:
 
   nsresult AddSupportsPrimitiveTojsvals(nsISupports *aArg, jsval *aArgv);
 
-  void FireGCTimer();
+  void FireGCTimer(PRBool aLoadInProgress);
 
   // given an nsISupports object (presumably an event target or some other
   // DOM object), get (or create) the JSObject wrapping it.

layout/base/Makefile.in

@@ -167,6 +167,7 @@ LOCAL_INCLUDES += \
 		-I$(srcdir)/../../content/events/src \
 		-I$(srcdir)/../../content/xbl/src \
 		-I$(srcdir)/../../view/src \
+		-I$(srcdir)/../../dom/src/base \
 		$(NULL)
 
 ifdef MOZ_MATHML

layout/base/nsDocumentViewer.cpp

@@ -118,6 +118,7 @@
 #include "nsIClipboardHelper.h"
 
 #include "nsPIDOMWindow.h"
+#include "nsJSEnvironment.h"
 #include "nsIFocusController.h"
 
 #include "nsIScrollableView.h"
@@ -512,8 +513,7 @@ DocumentViewerImpl::DocumentViewerImpl(nsPresContext* aPresContext)
   : mPresContext(aPresContext),
     mTextZoom(1.0),
     mIsSticky(PR_TRUE),
-    mHintCharsetSource(kCharsetUninitialized),
-    mIsPageMode(PR_FALSE)
+    mHintCharsetSource(kCharsetUninitialized)
 {
   PrepareToStartLoad();
 }
@@ -894,6 +894,8 @@ DocumentViewerImpl::InitInternal(nsIWidget* aParentWidget,
 
       if (window) {
         window->SetNewDocument(mDocument, aState, PR_TRUE);
+
+        nsJSContext::LoadStart();
       }
     }
   }
@@ -1109,6 +1111,8 @@ DocumentViewerImpl::LoadComplete(nsresult aStatus)
     DumpContentToPPM(name.get());
   }
 
+  nsJSContext::LoadEnd();
+
 #ifdef NS_PRINTING
   // Check to see if someone tried to print during the load
   if (mPrintIsPending) {