Bug 1348460 - make predictor::learn async. r=Ehsan
authorNicholas Hurley <hurley@mozilla.com>
Mon, 17 Apr 2017 17:22:46 -0700
changeset 354666 0107cfe404f802a735c4d040505d5d67b7cb6736
parent 354665 4ed3b236c64ca5d603b063aa353dca5826b276df
child 354667 1979070698f1faf642802fd1be5b7c5a51dd0657
push id31708
push userihsiao@mozilla.com
push dateTue, 25 Apr 2017 03:16:26 +0000
treeherdermozilla-central@f0621f7f0520 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1348460
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1348460 - make predictor::learn async. r=Ehsan Serializing and sending IPC messages takes a lot of time, and it gets in the way of image loading. Making this functionality async gets out of the way of image loading (among other things). The test has been changed to pump the main thread after calling predictor.learn so the multiprocess version can actually run to completion. This isn't strictly necessary in the single process version, but it makes the code changes (which are already pretty invasive) simpler. MozReview-Commit-ID: 7jvhomlygbf
netwerk/base/Predictor.cpp
netwerk/test/unit/test_predictor.js
--- a/netwerk/base/Predictor.cpp
+++ b/netwerk/base/Predictor.cpp
@@ -686,16 +686,57 @@ private:
 
     return rv;
   }
 
   nsCOMPtr<nsIThread> mIOThread;
   nsCOMPtr<nsIFile> mDBFile;
 };
 
+class PredictorLearnRunnable final : public Runnable {
+public:
+  PredictorLearnRunnable(nsIURI *targetURI, nsIURI *sourceURI,
+                         PredictorLearnReason reason, const OriginAttributes &oa)
+    : mTargetURI(targetURI)
+    , mSourceURI(sourceURI)
+    , mReason(reason)
+    , mOA(oa)
+  { }
+
+  ~PredictorLearnRunnable() { }
+
+  NS_IMETHOD Run() override
+  {
+    if (!gNeckoChild) {
+      // This may have gone away between when this runnable was dispatched and
+      // when it actually runs, so let's be safe here, even though we asserted
+      // earlier.
+      PREDICTOR_LOG(("predictor::learn (async) gNeckoChild went away"));
+      return NS_OK;
+    }
+
+    ipc::URIParams serTargetURI;
+    SerializeURI(mTargetURI, serTargetURI);
+
+    ipc::OptionalURIParams serSourceURI;
+    SerializeURI(mSourceURI, serSourceURI);
+
+    PREDICTOR_LOG(("predictor::learn (async) forwarding to parent"));
+    gNeckoChild->SendPredLearn(serTargetURI, serSourceURI, mReason, mOA);
+
+    return NS_OK;
+  }
+
+private:
+  nsCOMPtr<nsIURI> mTargetURI;
+  nsCOMPtr<nsIURI> mSourceURI;
+  PredictorLearnReason mReason;
+  const OriginAttributes mOA;
+};
+
 } // namespace
 
 void
 Predictor::MaybeCleanupOldDBFiles()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mEnabled || mCleanedUp) {
@@ -1501,25 +1542,20 @@ Predictor::LearnNative(nsIURI *targetURI
 
   PREDICTOR_LOG(("Predictor::Learn"));
 
   if (IsNeckoChild()) {
     MOZ_DIAGNOSTIC_ASSERT(gNeckoChild);
 
     PREDICTOR_LOG(("    called on child process"));
 
-    ipc::URIParams serTargetURI;
-    SerializeURI(targetURI, serTargetURI);
-
-    ipc::OptionalURIParams serSourceURI;
-    SerializeURI(sourceURI, serSourceURI);
-
-    PREDICTOR_LOG(("    forwarding to parent"));
-    gNeckoChild->SendPredLearn(serTargetURI, serSourceURI, reason,
-                               originAttributes);
+    RefPtr<PredictorLearnRunnable> runnable = new PredictorLearnRunnable(
+      targetURI, sourceURI, reason, originAttributes);
+    NS_DispatchToMainThread(runnable);
+
     return NS_OK;
   }
 
   PREDICTOR_LOG(("    called on parent process"));
 
   if (!mInitialized) {
     PREDICTOR_LOG(("    not initialized"));
     return NS_OK;
--- a/netwerk/test/unit/test_predictor.js
+++ b/netwerk/test/unit/test_predictor.js
@@ -210,25 +210,40 @@ function continue_test_pageload() {
   var subresources = [
     "http://localhost:4444/style.css",
     "http://localhost:4443/jquery.js",
     "http://localhost:4444/image.png"
   ];
 
   // This is necessary to learn the origin stuff
   predictor.learn(pageload_toplevel, null, predictor.LEARN_LOAD_TOPLEVEL, origin_attributes);
+  do_timeout(0, () => { // allow the learn() to run on the main thread
   var preconns = [];
-  for (var i = 0; i < subresources.length; i++) {
-    var sruri = newURI(subresources[i]);
-    predictor.learn(sruri, pageload_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
-    preconns.push(extract_origin(sruri));
-  }
+
+  var sruri = newURI(subresources[0]);
+  predictor.learn(sruri, pageload_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(sruri));
+
+  sruri = newURI(subresources[1]);
+  predictor.learn(sruri, pageload_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(sruri));
+
+  sruri = newURI(subresources[2]);
+  predictor.learn(sruri, pageload_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(sruri));
 
   var verifier = new Verifier("pageload", [], preconns, []);
   predictor.predict(pageload_toplevel, null, predictor.PREDICT_LOAD, origin_attributes, verifier);
+  });
+  });
+  });
+  });
 }
 
 function test_pageload() {
   open_and_continue([pageload_toplevel], function () {
     if (running_single_process) {
       continue_test_pageload();
     } else {
       sendCommand("continue_test_pageload();");
@@ -242,29 +257,48 @@ const redirect_targeturi = newURI("http:
 function continue_test_redrect() {
   var subresources = [
     "http://localhost:4444/style.css",
     "http://localhost:4443/jquery.js",
     "http://localhost:4444/image.png"
   ];
 
   predictor.learn(redirect_inituri, null, predictor.LEARN_LOAD_TOPLEVEL, origin_attributes);
+  do_timeout(0, () => {
   predictor.learn(redirect_targeturi, null, predictor.LEARN_LOAD_TOPLEVEL, origin_attributes);
+  do_timeout(0, () => {
   predictor.learn(redirect_targeturi, redirect_inituri, predictor.LEARN_LOAD_REDIRECT, origin_attributes);
+  do_tiemout(0, () => {
 
   var preconns = [];
   preconns.push(extract_origin(redirect_targeturi));
-  for (var i = 0; i < subresources.length; i++) {
-    var sruri = newURI(subresources[i]);
-    predictor.learn(sruri, redirect_targeturi, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
-    preconns.push(extract_origin(sruri));
-  }
+
+  var sruri = newURI(subresources[0]);
+  predictor.learn(sruri, redirect_targeturi, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(sruri));
+
+  sruri = newURI(subresources[1]);
+  predictor.learn(sruris[1], redirect_targeturi, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(sruri));
+
+  sruri = newURI(subresources[2]);
+  predictor.learn(sruris[2], redirect_targeturi, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(sruri));
 
   var verifier = new Verifier("redirect", [], preconns, []);
   predictor.predict(redirect_inituri, null, predictor.PREDICT_LOAD, origin_attributes, verifier);
+  });
+  });
+  });
+  });
+  });
+  });
 }
 
 function test_redirect() {
   open_and_continue([redirect_inituri, redirect_targeturi], function () {
     if (running_single_process) {
       continue_test_redirect();
     } else {
       sendCommand("continue_test_redirect();");
@@ -280,38 +314,48 @@ function test_startup() {
     return;
   }
 
   var uris = [
     "http://localhost:4444/startup",
     "http://localhost:4443/startup"
   ];
   var preconns = [];
-  for (var i = 0; i < uris.length; i++) {
-    var uri = newURI(uris[i]);
-    predictor.learn(uri, null, predictor.LEARN_STARTUP, origin_attributes);
-    preconns.push(extract_origin(uri));
-  }
+  var uri = newURI(uris[0]);
+  predictor.learn(uri, null, predictor.LEARN_STARTUP, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(uri));
+
+  uri = newURI(uris[1]);
+  predictor.learn(uri, null, predictor.LEARN_STARTUP, origin_attributes);
+  do_timeout(0, () => {
+  preconns.push(extract_origin(uri));
 
   var verifier = new Verifier("startup", [], preconns, []);
   predictor.predict(null, null, predictor.PREDICT_STARTUP, origin_attributes, verifier);
+  });
+  });
 }
 
 const dns_toplevel = newURI("http://localhost:4444/index.html");
 
 function continue_test_dns() {
   var subresource = "http://localhost:4443/jquery.js";
 
   predictor.learn(dns_toplevel, null, predictor.LEARN_LOAD_TOPLEVEL, origin_attributes);
+  do_timeout(0, () => {
   var sruri = newURI(subresource);
   predictor.learn(sruri, dns_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
 
   var preresolves = [extract_origin(sruri)];
   var verifier = new Verifier("dns", [], [], preresolves);
   predictor.predict(dns_toplevel, null, predictor.PREDICT_LOAD, origin_attributes, verifier);
+  });
+  });
 }
 
 function test_dns() {
   open_and_continue([dns_toplevel], function () {
     // Ensure that this will do preresolves
     Services.prefs.setIntPref("network.predictor.preconnect-min-confidence", 101);
     if (running_single_process) {
       continue_test_dns();
@@ -325,29 +369,50 @@ const origin_toplevel = newURI("http://l
 
 function continue_test_origin() {
   var subresources = [
     "http://localhost:4444/style.css",
     "http://localhost:4443/jquery.js",
     "http://localhost:4444/image.png"
   ];
   predictor.learn(origin_toplevel, null, predictor.LEARN_LOAD_TOPLEVEL, origin_attributes);
+  do_timeout(0, () => {
   var preconns = [];
-  for (var i = 0; i < subresources.length; i++) {
-    var sruri = newURI(subresources[i]);
-    predictor.learn(sruri, origin_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
-    var origin = extract_origin(sruri);
-    if (preconns.indexOf(origin) === -1) {
-      preconns.push(origin);
-    }
+
+  var sruri = newURI(subresources[0]);
+  predictor.learn(sruri, origin_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  var origin = extract_origin(sruri);
+  if (preconns.indexOf(origin) === -1) {
+    preconns.push(origin);
+  }
+
+  sruri = newURI(subresources[1]);
+  predictor.learn(sruri, origin_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  var origin = extract_origin(sruri);
+  if (preconns.indexOf(origin) === -1) {
+    preconns.push(origin);
+  }
+
+  sruri = newURI(subresources[2]);
+  predictor.learn(sruri, origin_toplevel, predictor.LEARN_LOAD_SUBRESOURCE, origin_attributes);
+  do_timeout(0, () => {
+  var origin = extract_origin(sruri);
+  if (preconns.indexOf(origin) === -1) {
+    preconns.push(origin);
   }
 
   var loaduri = newURI("http://localhost:4444/anotherpage.html");
   var verifier = new Verifier("origin", [], preconns, []);
   predictor.predict(loaduri, null, predictor.PREDICT_LOAD, origin_attributes, verifier);
+  });
+  });
+  });
+  });
 }
 
 function test_origin() {
   open_and_continue([origin_toplevel], function () {
     if (running_single_process) {
       continue_test_origin();
     } else {
       sendCommand("continue_test_origin();");