Bug 1509441: Introduce a factory function to build immutable CompileArgs; r=lth
authorBenjamin Bouvier <benj@benj.me>
Wed, 23 Jan 2019 15:09:16 +0100
changeset 515299 677eda6bdaf96eab98a43cfe4e4ab47d53f9e29c
parent 515298 48dc14f79fb0a51ca796257a4179fe6f16b71b14
child 515300 cb20dcd8ea7e8e7a20392cfb7bf3759c26cfc240
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslth
bugs1509441
milestone66.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 1509441: Introduce a factory function to build immutable CompileArgs; r=lth
js/src/wasm/AsmJS.cpp
js/src/wasm/WasmCompile.cpp
js/src/wasm/WasmCompile.h
js/src/wasm/WasmJS.cpp
--- a/js/src/wasm/AsmJS.cpp
+++ b/js/src/wasm/AsmJS.cpp
@@ -2132,18 +2132,17 @@ class MOZ_STACK_CLASS JS_HAZ_ROOTED Modu
     if (parser_.ss->filename()) {
       scriptedCaller.line = 0;  // unused
       scriptedCaller.filename = DuplicateString(parser_.ss->filename());
       if (!scriptedCaller.filename) {
         return nullptr;
       }
     }
 
-    MutableCompileArgs args =
-        cx_->new_<CompileArgs>(cx_, std::move(scriptedCaller));
+    SharedCompileArgs args = CompileArgs::build(cx_, std::move(scriptedCaller));
     if (!args) {
       return nullptr;
     }
 
     uint32_t codeSectionSize = 0;
     for (const Func& func : funcDefs_) {
       codeSectionSize += func.bytes().length();
     }
--- a/js/src/wasm/WasmCompile.cpp
+++ b/js/src/wasm/WasmCompile.cpp
@@ -67,38 +67,46 @@ uint32_t wasm::ObservedCPUFeatures() {
   return MIPS64 | (jit::GetMIPSFlags() << ARCH_BITS);
 #elif defined(JS_CODEGEN_NONE)
   return 0;
 #else
 #  error "unknown architecture"
 #endif
 }
 
-CompileArgs::CompileArgs(JSContext* cx, ScriptedCaller&& scriptedCaller)
-    : scriptedCaller(std::move(scriptedCaller)) {
-  baselineEnabled = cx->options().wasmBaseline();
-  ionEnabled = cx->options().wasmIon();
+SharedCompileArgs
+CompileArgs::build(JSContext* cx, ScriptedCaller&& scriptedCaller)
+{
+  CompileArgs* target = cx->new_<CompileArgs>(std::move(scriptedCaller));
+  if (!target) {
+    return nullptr;
+  }
+
+  target->baselineEnabled = cx->options().wasmBaseline();
+  target->ionEnabled = cx->options().wasmIon();
 #ifdef ENABLE_WASM_CRANELIFT
-  craneliftEnabled = cx->options().wasmCranelift();
+  target->craneliftEnabled = cx->options().wasmCranelift();
 #else
-  craneliftEnabled = false;
+  target->craneliftEnabled = false;
 #endif
 
   // Debug information such as source view or debug traps will require
   // additional memory and permanently stay in baseline code, so we try to
   // only enable it when a developer actually cares: when the debugger tab
   // is open.
-  debugEnabled = cx->realm()->debuggerObservesAsmJS();
+  target->debugEnabled = cx->realm()->debuggerObservesAsmJS();
 
-  sharedMemoryEnabled =
+  target->sharedMemoryEnabled =
       cx->realm()->creationOptions().getSharedMemoryAndAtomicsEnabled();
-  forceTiering =
+  target->forceTiering =
       cx->options().testWasmAwaitTier2() || JitOptions.wasmDelayTier2;
 
-  gcEnabled = HasReftypesSupport(cx);
+  target->gcEnabled = HasReftypesSupport(cx);
+
+  return target;
 }
 
 // Classify the current system as one of a set of recognizable classes.  This
 // really needs to get our tier-1 systems right.
 //
 // TODO: We don't yet have a good measure of how fast a system is.  We
 // distinguish between mobile and desktop because these are very different kinds
 // of systems, but we could further distinguish between low / medium / high end
--- a/js/src/wasm/WasmCompile.h
+++ b/js/src/wasm/WasmCompile.h
@@ -37,44 +37,54 @@ struct ScriptedCaller {
   bool filenameIsURL;
   unsigned line;
 
   ScriptedCaller() : filenameIsURL(false), line(0) {}
 };
 
 // Describes all the parameters that control wasm compilation.
 
+struct CompileArgs;
+typedef RefPtr<CompileArgs> MutableCompileArgs;
+typedef RefPtr<const CompileArgs> SharedCompileArgs;
+
 struct CompileArgs : ShareableBase<CompileArgs> {
   ScriptedCaller scriptedCaller;
   UniqueChars sourceMapURL;
 
   bool baselineEnabled;
   bool ionEnabled;
   bool craneliftEnabled;
   bool debugEnabled;
   bool sharedMemoryEnabled;
   bool forceTiering;
   bool gcEnabled;
 
+  // CompileArgs has two constructors:
+  //
+  // - one through a factory function `build`, which checks that flags are
+  // consistent with each other.
+  // - one that gives complete access to underlying fields.
+  //
+  // You should use the first one in general, unless you have a very good
+  // reason (i.e. no JSContext around and you know which flags have been used).
+
+  static SharedCompileArgs build(JSContext* cx, ScriptedCaller&& scriptedCaller);
+
   explicit CompileArgs(ScriptedCaller&& scriptedCaller)
       : scriptedCaller(std::move(scriptedCaller)),
         baselineEnabled(false),
         ionEnabled(false),
         craneliftEnabled(false),
         debugEnabled(false),
         sharedMemoryEnabled(false),
         forceTiering(false),
         gcEnabled(false) {}
-
-  CompileArgs(JSContext* cx, ScriptedCaller&& scriptedCaller);
 };
 
-typedef RefPtr<CompileArgs> MutableCompileArgs;
-typedef RefPtr<const CompileArgs> SharedCompileArgs;
-
 // Return the estimated compiled (machine) code size for the given bytecode size
 // compiled at the given tier.
 
 double EstimateCompiledCodeSize(Tier tier, size_t bytecodeSize);
 
 // Compile the given WebAssembly bytecode with the given arguments into a
 // wasm::Module. On success, the Module is returned. On failure, the returned
 // SharedModule pointer is null and either:
--- a/js/src/wasm/WasmJS.cpp
+++ b/js/src/wasm/WasmJS.cpp
@@ -404,18 +404,17 @@ bool wasm::Eval(JSContext* cx, Handle<Ty
     return false;
   }
 
   ScriptedCaller scriptedCaller;
   if (!DescribeScriptedCaller(cx, &scriptedCaller, "wasm_eval")) {
     return false;
   }
 
-  MutableCompileArgs compileArgs =
-      cx->new_<CompileArgs>(cx, std::move(scriptedCaller));
+  SharedCompileArgs compileArgs = CompileArgs::build(cx, std::move(scriptedCaller));
   if (!compileArgs) {
     return false;
   }
 
   UniqueChars error;
   UniqueCharsVector warnings;
   SharedModule module =
       CompileBuffer(*compileArgs, *bytecode, &error, &warnings);
@@ -1027,24 +1026,24 @@ static bool GetBufferSource(JSContext* c
   if (!(*bytecode)->append(dataPointer.unwrap(), byteLength)) {
     ReportOutOfMemory(cx);
     return false;
   }
 
   return true;
 }
 
-static MutableCompileArgs InitCompileArgs(JSContext* cx,
-                                          const char* introducer) {
+static SharedCompileArgs InitCompileArgs(JSContext* cx,
+                                         const char* introducer) {
   ScriptedCaller scriptedCaller;
   if (!DescribeScriptedCaller(cx, &scriptedCaller, introducer)) {
     return nullptr;
   }
 
-  return cx->new_<CompileArgs>(cx, std::move(scriptedCaller));
+  return CompileArgs::build(cx, std::move(scriptedCaller));
 }
 
 static bool ReportCompileWarnings(JSContext* cx,
                                   const UniqueCharsVector& warnings) {
   // Avoid spamming the console.
   size_t numWarnings = Min<size_t>(warnings.length(), 3);
 
   for (size_t i = 0; i < numWarnings; i++) {
@@ -3333,29 +3332,29 @@ class ResolveResponseClosure : public Na
   static void finalize(FreeOp* fop, JSObject* obj) {
     obj->as<ResolveResponseClosure>().compileArgs().Release();
   }
 
  public:
   static const unsigned RESERVED_SLOTS = 4;
   static const Class class_;
 
-  static ResolveResponseClosure* create(JSContext* cx, CompileArgs& args,
+  static ResolveResponseClosure* create(JSContext* cx, const CompileArgs& args,
                                         HandleObject promise, bool instantiate,
                                         HandleObject importObj) {
     MOZ_ASSERT_IF(importObj, instantiate);
 
     AutoSetNewObjectMetadata metadata(cx);
     auto* obj = NewObjectWithGivenProto<ResolveResponseClosure>(cx, nullptr);
     if (!obj) {
       return nullptr;
     }
 
     args.AddRef();
-    obj->setReservedSlot(COMPILE_ARGS_SLOT, PrivateValue(&args));
+    obj->setReservedSlot(COMPILE_ARGS_SLOT, PrivateValue((void*)&args));
     obj->setReservedSlot(PROMISE_OBJ_SLOT, ObjectValue(*promise));
     obj->setReservedSlot(INSTANTIATE_SLOT, BooleanValue(instantiate));
     obj->setReservedSlot(IMPORT_OBJ_SLOT, ObjectOrNullValue(importObj));
     return obj;
   }
 
   CompileArgs& compileArgs() const {
     return *(CompileArgs*)getReservedSlot(COMPILE_ARGS_SLOT).toPrivate();
@@ -3454,17 +3453,17 @@ static bool ResolveResponse(JSContext* c
                             Handle<PromiseObject*> promise,
                             bool instantiate = false,
                             HandleObject importObj = nullptr) {
   MOZ_ASSERT_IF(importObj, instantiate);
 
   const char* introducer = instantiate ? "WebAssembly.instantiateStreaming"
                                        : "WebAssembly.compileStreaming";
 
-  MutableCompileArgs compileArgs = InitCompileArgs(cx, introducer);
+  SharedCompileArgs compileArgs = InitCompileArgs(cx, introducer);
   if (!compileArgs) {
     return false;
   }
 
   RootedObject closure(
       cx, ResolveResponseClosure::create(cx, *compileArgs, promise, instantiate,
                                          importObj));
   if (!closure) {