Bug 1547721 - Add an Ion script size limit. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 29 Apr 2019 15:28:53 +0000
changeset 471767 c0101502b8b76ad563a3e84b5df203586394f64d
parent 471766 4b11fe46132ced08322ca986ec9c56c15e862c06
child 471768 47c353bd446dc5ae4e4a642879e7eb46c3bd5e21
push id35934
push usershindli@mozilla.com
push dateMon, 29 Apr 2019 21:53:38 +0000
treeherdermozilla-central@f6766ba4ac77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1547721
milestone68.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 1547721 - Add an Ion script size limit. r=tcampbell We had a script size limit (and number of locals/args limit) when off-thread Ion compilation wasn't available, but no hard limit when off-thread compilation is available. This patch addresses that. This patch also converts the main-thread limits from constants to JitOptions because that's what we use for new code. Differential Revision: https://phabricator.services.mozilla.com/D29217
js/src/jit/Ion.cpp
js/src/jit/IonOptimizationLevels.cpp
js/src/jit/JitOptions.cpp
js/src/jit/JitOptions.h
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -2153,26 +2153,31 @@ static bool CanIonCompileOrInlineScript(
   return true;
 }
 
 static bool ScriptIsTooLarge(JSContext* cx, JSScript* script) {
   if (!JitOptions.limitScriptSize) {
     return false;
   }
 
-  uint32_t numLocalsAndArgs = NumLocalsAndArgs(script);
-
-  if (script->length() > MAX_MAIN_THREAD_SCRIPT_SIZE ||
-      numLocalsAndArgs > MAX_MAIN_THREAD_LOCALS_AND_ARGS) {
-    if (!OffThreadCompilationAvailable(cx)) {
-      JitSpew(JitSpew_IonAbort, "Script too large (%zu bytes) (%u locals/args)",
-              script->length(), numLocalsAndArgs);
-      TrackIonAbort(cx, script, script->code(), "too large");
-      return true;
-    }
+  size_t numLocalsAndArgs = NumLocalsAndArgs(script);
+
+  bool canCompileOffThread = OffThreadCompilationAvailable(cx);
+  size_t maxScriptSize = canCompileOffThread
+                             ? JitOptions.ionMaxScriptSize
+                             : JitOptions.ionMaxScriptSizeMainThread;
+  size_t maxLocalsAndArgs = canCompileOffThread
+                                ? JitOptions.ionMaxLocalsAndArgs
+                                : JitOptions.ionMaxLocalsAndArgsMainThread;
+
+  if (script->length() > maxScriptSize || numLocalsAndArgs > maxLocalsAndArgs) {
+    JitSpew(JitSpew_IonAbort, "Script too large (%zu bytes) (%zu locals/args)",
+            script->length(), numLocalsAndArgs);
+    TrackIonAbort(cx, script, script->code(), "too large");
+    return true;
   }
 
   return false;
 }
 
 bool CanIonCompileScript(JSContext* cx, JSScript* script) {
   if (!script->canIonCompile()) {
     return false;
--- a/js/src/jit/IonOptimizationLevels.cpp
+++ b/js/src/jit/IonOptimizationLevels.cpp
@@ -88,24 +88,25 @@ uint32_t OptimizationInfo::compilerWarmU
 
   uint32_t warmUpThreshold = baseCompilerWarmUpThreshold();
 
   // If the script is too large to compile on the main thread, we can still
   // compile it off thread. In these cases, increase the warm-up counter
   // threshold to improve the compilation's type information and hopefully
   // avoid later recompilation.
 
-  if (script->length() > MAX_MAIN_THREAD_SCRIPT_SIZE) {
-    warmUpThreshold *= (script->length() / double(MAX_MAIN_THREAD_SCRIPT_SIZE));
+  if (script->length() > JitOptions.ionMaxScriptSizeMainThread) {
+    warmUpThreshold *=
+        (script->length() / double(JitOptions.ionMaxScriptSizeMainThread));
   }
 
   uint32_t numLocalsAndArgs = NumLocalsAndArgs(script);
-  if (numLocalsAndArgs > MAX_MAIN_THREAD_LOCALS_AND_ARGS) {
+  if (numLocalsAndArgs > JitOptions.ionMaxLocalsAndArgsMainThread) {
     warmUpThreshold *=
-        (numLocalsAndArgs / double(MAX_MAIN_THREAD_LOCALS_AND_ARGS));
+        (numLocalsAndArgs / double(JitOptions.ionMaxLocalsAndArgsMainThread));
   }
 
   if (!pc || JitOptions.eagerIonCompilation()) {
     return warmUpThreshold;
   }
 
   // It's more efficient to enter outer loops, rather than inner loops, via OSR.
   // To accomplish this, we use a slightly higher threshold for inner loops.
--- a/js/src/jit/JitOptions.cpp
+++ b/js/src/jit/JitOptions.cpp
@@ -207,16 +207,24 @@ DefaultJitOptions::DefaultJitOptions() {
   // score is compared against a threshold to prevent a branch from being
   // removed.
   SET_DEFAULT(branchPruningHitCountFactor, 1);
   SET_DEFAULT(branchPruningInstFactor, 10);
   SET_DEFAULT(branchPruningBlockSpanFactor, 100);
   SET_DEFAULT(branchPruningEffectfulInstFactor, 3500);
   SET_DEFAULT(branchPruningThreshold, 4000);
 
+  // Limits on bytecode length and number of locals/arguments for Ion
+  // compilation. There are different (lower) limits for when off-thread Ion
+  // compilation isn't available.
+  SET_DEFAULT(ionMaxScriptSize, 100 * 1000);
+  SET_DEFAULT(ionMaxScriptSizeMainThread, 2 * 1000);
+  SET_DEFAULT(ionMaxLocalsAndArgs, 10 * 1000);
+  SET_DEFAULT(ionMaxLocalsAndArgsMainThread, 256);
+
   // Force the used register allocator instead of letting the optimization
   // pass decide.
   const char* forcedRegisterAllocatorEnv = "JIT_OPTION_forcedRegisterAllocator";
   if (const char* env = getenv(forcedRegisterAllocatorEnv)) {
     forcedRegisterAllocator = LookupRegisterAllocator(env);
     if (!forcedRegisterAllocator.isSome()) {
       Warn(forcedRegisterAllocatorEnv, env);
     }
--- a/js/src/jit/JitOptions.h
+++ b/js/src/jit/JitOptions.h
@@ -10,21 +10,16 @@
 #include "mozilla/Maybe.h"
 
 #include "jit/IonTypes.h"
 #include "js/TypeDecls.h"
 
 namespace js {
 namespace jit {
 
-// Longer scripts can only be compiled off thread, as these compilations
-// can be expensive and stall the main thread for too long.
-static const uint32_t MAX_MAIN_THREAD_SCRIPT_SIZE = 2 * 1000;
-static const uint32_t MAX_MAIN_THREAD_LOCALS_AND_ARGS = 256;
-
 // Possible register allocators which may be used.
 enum IonRegisterAllocator {
   RegisterAllocator_Backtracking,
   RegisterAllocator_Testbed,
   RegisterAllocator_Stupid
 };
 
 static inline mozilla::Maybe<IonRegisterAllocator> LookupRegisterAllocator(
@@ -93,16 +88,20 @@ struct DefaultJitOptions {
   uint32_t osrPcMismatchesBeforeRecompile;
   uint32_t smallFunctionMaxBytecodeLength_;
   uint32_t jumpThreshold;
   uint32_t branchPruningHitCountFactor;
   uint32_t branchPruningInstFactor;
   uint32_t branchPruningBlockSpanFactor;
   uint32_t branchPruningEffectfulInstFactor;
   uint32_t branchPruningThreshold;
+  uint32_t ionMaxScriptSize;
+  uint32_t ionMaxScriptSizeMainThread;
+  uint32_t ionMaxLocalsAndArgs;
+  uint32_t ionMaxLocalsAndArgsMainThread;
   uint32_t wasmBatchIonThreshold;
   uint32_t wasmBatchBaselineThreshold;
   mozilla::Maybe<IonRegisterAllocator> forcedRegisterAllocator;
 
   // Spectre mitigation flags. Each mitigation has its own flag in order to
   // measure the effectiveness of each mitigation with various proof of
   // concept.
   bool spectreIndexMasking;