Bug 900784 - Tune the bytecode cache heuristic based on telemetry results. r=mrbkap
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Fri, 28 Jul 2017 13:06:48 +0000
changeset 422823 d2cf47e965d7898f0179512875e6e6c877174ef3
parent 422822 816e61957699d19eb96dfccd014ca0487337f065
child 422824 d578eb28a8e2363f01f3afc0a51ea5aa49f65768
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs900784
milestone56.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 900784 - Tune the bytecode cache heuristic based on telemetry results. r=mrbkap
dom/script/ScriptLoader.cpp
modules/libpref/init/all.js
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -1923,94 +1923,40 @@ ScriptLoader::ShouldCacheBytecode(Script
 
   // Look at the preference to know which strategy (parameters) should be used
   // when the bytecode cache is enabled.
   int32_t strategy = nsContentUtils::BytecodeCacheStrategy();
 
   // List of parameters used by the strategies.
   bool hasSourceLengthMin = false;
   bool hasFetchCountMin = false;
-  bool hasTimeSinceLastFetched = false;
   size_t sourceLengthMin = 100;
-  int32_t fetchCountMin = 5;
-  TimeDuration timeSinceLastFetched;
+  int32_t fetchCountMin = 4;
 
   LOG(("ScriptLoadRequest (%p): Bytecode-cache: strategy = %d.", aRequest, strategy));
   switch (strategy) {
     case -2: {
       // Reader mode, keep requesting alternate data but no longer save it.
       LOG(("ScriptLoadRequest (%p): Bytecode-cache: Encoding disabled.", aRequest));
       return false;
     }
     case -1: {
       // Eager mode, skip heuristics!
       hasSourceLengthMin = false;
       hasFetchCountMin = false;
-      hasTimeSinceLastFetched = false;
       break;
     }
     default:
     case 0: {
       hasSourceLengthMin = true;
       hasFetchCountMin = true;
-      hasTimeSinceLastFetched = true;
       sourceLengthMin = 1024;
-      fetchCountMin = 5;
-      timeSinceLastFetched = TimeDuration::FromSeconds(72 * 3600);
-      break;
-    }
-
-    // The following strategies are made-up to study what impact each parameter
-    // has when compared to the default case.
-    case 1: {
-      hasSourceLengthMin = true;
-      hasFetchCountMin = true;
-      hasTimeSinceLastFetched = false;
-      sourceLengthMin = 1024;
-      fetchCountMin = 5;
-      break;
-    }
-    case 2: {
-      hasSourceLengthMin = true;
-      hasFetchCountMin = false;
-      hasTimeSinceLastFetched = true;
-      sourceLengthMin = 1024;
-      timeSinceLastFetched = TimeDuration::FromSeconds(72 * 3600);
-      break;
-    }
-    case 3: {
-      hasSourceLengthMin = false;
-      hasFetchCountMin = true;
-      hasTimeSinceLastFetched = true;
-      fetchCountMin = 5;
-      timeSinceLastFetched = TimeDuration::FromSeconds(72 * 3600);
-      break;
-    }
-
-    // The following strategies are made-up to study what impact each parameter
-    // has individually.
-    case 4: {
-      hasSourceLengthMin = false;
-      hasFetchCountMin = false;
-      hasTimeSinceLastFetched = true;
-      timeSinceLastFetched = TimeDuration::FromSeconds(72 * 3600);
-      break;
-    }
-    case 5: {
-      hasSourceLengthMin = false;
-      hasFetchCountMin = true;
-      hasTimeSinceLastFetched = false;
-      fetchCountMin = 5;
-      break;
-    }
-    case 6: {
-      hasSourceLengthMin = true;
-      hasFetchCountMin = false;
-      hasTimeSinceLastFetched = false;
-      sourceLengthMin = 1024;
+      // If we were to optimize only for speed, without considering the impact
+      // on memory, we should set this threshold to 2. (Bug 900784 comment 120)
+      fetchCountMin = 4;
       break;
     }
   }
 
   // If the script is too small/large, do not attempt at creating a bytecode
   // cache for this script, as the overhead of parsing it might not be worth the
   // effort.
   if (hasSourceLengthMin && aRequest->mScriptText.length() < sourceLengthMin) {
@@ -2028,36 +1974,16 @@ ScriptLoader::ShouldCacheBytecode(Script
       return false;
     }
     LOG(("ScriptLoadRequest (%p): Bytecode-cache: fetchCount = %d.", aRequest, fetchCount));
     if (fetchCount < fetchCountMin) {
       return false;
     }
   }
 
-  // Check that the cache entry got accessed recently, before caching it.
-  if (hasTimeSinceLastFetched) {
-    uint32_t lastFetched = 0;
-    if (NS_FAILED(aRequest->mCacheInfo->GetCacheTokenLastFetched(&lastFetched))) {
-      LOG(("ScriptLoadRequest (%p): Bytecode-cache: Cannot get lastFetched.", aRequest));
-      return false;
-    }
-    uint32_t now = PR_Now() / PR_USEC_PER_SEC;
-    if (now < lastFetched) {
-      LOG(("ScriptLoadRequest (%p): Bytecode-cache: (What?) lastFetched set in the future.", aRequest));
-      return false;
-    }
-    TimeDuration since = TimeDuration::FromSeconds(now - lastFetched);
-    LOG(("ScriptLoadRequest (%p): Bytecode-cache: lastFetched = %f sec. ago.",
-         aRequest, since.ToSeconds()));
-    if (since >= timeSinceLastFetched) {
-      return false;
-    }
-  }
-
   LOG(("ScriptLoadRequest (%p): Bytecode-cache: Trigger encoding.", aRequest));
   return true;
 }
 
 nsresult
 ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest)
 {
   using namespace mozilla::Telemetry;
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -213,17 +213,17 @@ pref("dom.keyboardevent.dispatch_during_
 // causes a separate compartment for each (addon, global) combination, which may
 // significantly increase the number of compartments in the system.
 pref("dom.compartment_per_addon", true);
 
 // Whether to enable the JavaScript start-up cache. This causes one of the first
 // execution to record the bytecode of the JavaScript function used, and save it
 // in the existing cache entry. On the following loads of the same script, the
 // bytecode would be loaded from the cache instead of being generated once more.
-pref("dom.script_loader.bytecode_cache.enabled", false); // Not tuned yet.
+pref("dom.script_loader.bytecode_cache.enabled", false);
 
 // Ignore the heuristics of the bytecode cache, and always record on the first
 // visit. (used for testing purposes).
 
 // Choose one strategy to use to decide when the bytecode should be encoded and
 // saved. The following strategies are available right now:
 //   * -2 : (reader mode) The bytecode cache would be read, but it would never
 //          be saved.