Separate module instantiate counter for asm and wasm.

src/wasm/wasm-module.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <memory>
 
 #include "src/assembler-inl.h"
 #include "src/base/adapters.h"
 #include "src/base/atomic-utils.h"
 #include "src/code-stubs.h"
@@ skipping 1135 matching lines @@
   // Build an instance, in all of its glory.
   MaybeHandle<WasmInstanceObject> Build() {
     // Check that an imports argument was provided, if the module requires it.
     // No point in continuing otherwise.
     if (!module_->import_table.empty() && ffi_.is_null()) {
       thrower_->TypeError(
           "Imports argument must be present and must be an object");
       return {};
     }
 
-    HistogramTimerScope wasm_instantiate_module_time_scope(
-        isolate_->counters()->wasm_instantiate_module_time());
+    // Record build time into correct bucket, then build instance.
+    if (module_->origin == ModuleOrigin::kWasmOrigin) {
+      HistogramTimerScope wasm_instantiate_module_time_scope(
+          isolate_->counters()->wasm_instantiate_wasm_module_time());
+      return BuildInstance();
+    } else {
+      HistogramTimerScope wasm_instantiate_module_time_scope(
+          isolate_->counters()->wasm_instantiate_asm_module_time());
+      return BuildInstance();
+    }

[bbudge, 2017/03/23 22:04:58]

Why not return BuildInstance() here? Or why not just inline that code like it
was before? It would make the patch a lot easier to review :)

[bbudge, 2017/03/23 22:14:48]

I didn't see the scope objects before. You could construct a single scope object
using asm/wasm condition to route the right counter to it.

[Karl, 2017/03/24 16:49:19]

I factored out the rest of the code, as you note, to take advantage of the scope
object. However, you are also right that I could simplify the code by moving the
conditional into the argument passed to the scope.

Fixing.

+  }
+
+ private:
+  // Represents the initialized state of a table.
+  struct TableInstance {
+    Handle<WasmTableObject> table_object;  // WebAssembly.Table instance
+    Handle<FixedArray> js_wrappers;        // JSFunctions exported
+    Handle<FixedArray> function_table;     // internal code array
+    Handle<FixedArray> signature_table;    // internal sig array
+  };
+
+  Isolate* isolate_;
+  WasmModule* const module_;
+  ErrorThrower* thrower_;
+  Handle<WasmModuleObject> module_object_;
+  Handle<JSReceiver> ffi_;        // TODO(titzer): Use MaybeHandle
+  Handle<JSArrayBuffer> memory_;  // TODO(titzer): Use MaybeHandle
+  Handle<JSArrayBuffer> globals_;
+  Handle<WasmCompiledModule> compiled_module_;
+  std::vector<TableInstance> table_instances_;
+  std::vector<Handle<JSFunction>> js_wrappers_;
+  JSToWasmWrapperCache js_to_wasm_cache_;
+
+  // Helper routines to print out errors with imports.
+  void ReportLinkError(const char* error, uint32_t index,
+                       Handle<String> module_name, Handle<String> import_name) {
+    thrower_->LinkError(
+        "Import #%d module=\"%.*s\" function=\"%.*s\" error: %s", index,
+        module_name->length(), module_name->ToCString().get(),
+        import_name->length(), import_name->ToCString().get(), error);
+  }
+
+  MaybeHandle<Object> ReportLinkError(const char* error, uint32_t index,
+                                      Handle<String> module_name) {
+    thrower_->LinkError("Import #%d module=\"%.*s\" error: %s", index,
+                        module_name->length(), module_name->ToCString().get(),
+                        error);
+    return MaybeHandle<Object>();
+  }
+
+  // Build an instance, in all its glory.
+  MaybeHandle<WasmInstanceObject> BuildInstance() {
     Factory* factory = isolate_->factory();
 
     //--------------------------------------------------------------------------
     // Reuse the compiled module (if no owner), otherwise clone.
     //--------------------------------------------------------------------------
     Handle<FixedArray> code_table;
     Handle<FixedArray> old_code_table;
     MaybeHandle<WasmInstanceObject> owner;
 
     TRACE("Starting new module instantiation\n");
@@ skipping 349 matching lines @@
         return {};
       }
     }
 
     DCHECK(!isolate_->has_pending_exception());
     TRACE("Finishing instance %d\n", compiled_module_->instance_id());
     TRACE_CHAIN(module_object_->compiled_module());
     return instance;
   }
 
- private:
-  // Represents the initialized state of a table.
-  struct TableInstance {
-    Handle<WasmTableObject> table_object;    // WebAssembly.Table instance
-    Handle<FixedArray> js_wrappers;          // JSFunctions exported
-    Handle<FixedArray> function_table;       // internal code array
-    Handle<FixedArray> signature_table;      // internal sig array
-  };
-
-  Isolate* isolate_;
-  WasmModule* const module_;
-  ErrorThrower* thrower_;
-  Handle<WasmModuleObject> module_object_;
-  Handle<JSReceiver> ffi_;        // TODO(titzer): Use MaybeHandle
-  Handle<JSArrayBuffer> memory_;  // TODO(titzer): Use MaybeHandle
-  Handle<JSArrayBuffer> globals_;
-  Handle<WasmCompiledModule> compiled_module_;
-  std::vector<TableInstance> table_instances_;
-  std::vector<Handle<JSFunction>> js_wrappers_;
-  JSToWasmWrapperCache js_to_wasm_cache_;
-
-  // Helper routines to print out errors with imports.
-  void ReportLinkError(const char* error, uint32_t index,
-                       Handle<String> module_name, Handle<String> import_name) {
-    thrower_->LinkError(
-        "Import #%d module=\"%.*s\" function=\"%.*s\" error: %s", index,
-        module_name->length(), module_name->ToCString().get(),
-        import_name->length(), import_name->ToCString().get(), error);
-  }
-
-  MaybeHandle<Object> ReportLinkError(const char* error, uint32_t index,
-                                      Handle<String> module_name) {
-    thrower_->LinkError("Import #%d module=\"%.*s\" error: %s", index,
-                        module_name->length(), module_name->ToCString().get(),
-                        error);
-    return MaybeHandle<Object>();
-  }
-
   // Look up an import value in the {ffi_} object.
   MaybeHandle<Object> LookupImport(uint32_t index, Handle<String> module_name,
                                    Handle<String> import_name) {
     // We pre-validated in the js-api layer that the ffi object is present, and
     // a JSObject, if the module has imports.
     DCHECK(!ffi_.is_null());
 
     // Look up the module first.
     MaybeHandle<Object> result =
         Object::GetPropertyOrElement(ffi_, module_name);
@@ skipping 1627 matching lines @@
                                      callee_compiled->instruction_start());
     }
     DCHECK_EQ(non_compiled_functions.size(), idx);
   }
 
   Code* ret =
       Code::cast(compiled_module->code_table()->get(func_to_return_idx));
   DCHECK_EQ(Code::WASM_FUNCTION, ret->kind());
   return handle(ret, isolate);
 }