src/wasm/module-compiler.cc - Issue 2929853003: Fix use of history timers in background threads.

Unified Diff: src/wasm/module-compiler.cc

Issue 2929853003: Fix use of history timers in background threads. (Closed)

Patch Set: Merge with master Created 3 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: src/wasm/module-compiler.cc

diff --git a/src/wasm/module-compiler.cc b/src/wasm/module-compiler.cc

index 4a06179d818402f4aa96344c33562e86e4382815..6dccbaf0e66e16b318971dca2a81e94d2a6f9083 100644

--- a/src/wasm/module-compiler.cc

+++ b/src/wasm/module-compiler.cc

@@ -97,7 +97,6 @@ ModuleCompiler::ModuleCompiler(Isolate* isolate,

Min(static_cast<size_t>(FLAG_wasm_num_compilation_tasks),

V8::GetCurrentPlatform()->NumberOfAvailableBackgroundThreads())),

stopped_compilation_tasks_(num_background_tasks_) {

- counters_ = counters_shared_.get();

}

bool ModuleCompiler::GetNextUncompiledFunctionId(size_t* index) {

@@ -184,10 +183,8 @@ size_t ModuleCompiler::InitializeParallelCompilation(

compilation_units_.reserve(funcs_to_compile);

for (uint32_t i = start; i < num_funcs; ++i) {

const WasmFunction* func = &functions[i];

- constexpr bool is_sync = true;

compilation_units_.push_back(std::unique_ptr<compiler::WasmCompilationUnit>(

- new compiler::WasmCompilationUnit(isolate_, &module_env, func,

- !is_sync)));

+ new compiler::WasmCompilationUnit(isolate_, &module_env, func)));

}

return funcs_to_compile;

}

@@ -365,18 +362,9 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObject(

function_tables->set(i, *temp_instance.function_tables[i]);

signature_tables->set(i, *temp_instance.signature_tables[i]);

}

- if (is_sync_) {

- // TODO(karlschimpf): Make this work when asynchronous.

- // https://bugs.chromium.org/p/v8/issues/detail?id=6361

- HistogramTimerScope wasm_compile_module_time_scope(

- module_->is_wasm()

- ? isolate_->counters()->wasm_compile_wasm_module_time()

- : isolate_->counters()->wasm_compile_asm_module_time());

- return CompileToModuleObjectInternal(

- thrower, wire_bytes, asm_js_script, asm_js_offset_table_bytes, factory,

- &temp_instance, &function_tables, &signature_tables);

- }

+ TimedHistogramScope wasm_compile_module_time_scope(

+ module_->is_wasm() ? counters()->wasm_compile_wasm_module_time()

Mircea Trofin 2017/06/22 18:42:24 can this test be encapsulated in a method, it appe

kschimpf 2017/06/22 20:38:31 When I tried that in an earlier review, there was

+ : counters()->wasm_compile_asm_module_time());

return CompileToModuleObjectInternal(

thrower, wire_bytes, asm_js_script, asm_js_offset_table_bytes, factory,

&temp_instance, &function_tables, &signature_tables);

@@ -615,8 +603,8 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(

if (is_sync_)

// TODO(karlschimpf): Make this work when asynchronous.

// https://bugs.chromium.org/p/v8/issues/detail?id=6361

- (module_->is_wasm() ? isolate_->counters()->wasm_functions_per_wasm_module()

- : isolate_->counters()->wasm_functions_per_asm_module())

+ (module_->is_wasm() ? counters()->wasm_functions_per_wasm_module()

+ : counters()->wasm_functions_per_asm_module())

->AddSample(static_cast<int>(module_->functions.size()));

if (!lazy_compile) {

@@ -650,7 +638,7 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(

i < temp_instance->function_code.size(); ++i) {

Code* code = *temp_instance->function_code[i];

code_table->set(static_cast<int>(i), code);

- RecordStats(code, counters_);

+ RecordStats(code, counters());

}

// Create heap objects for script, module bytes and asm.js offset table to

@@ -716,7 +704,7 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(

isolate_, module, wasm_code, exp.index);

int export_index = static_cast<int>(module->functions.size() + func_index);

code_table->set(export_index, *wrapper_code);

- RecordStats(*wrapper_code, counters_);

+ RecordStats(*wrapper_code, counters());

func_index++;

}

@@ -769,7 +757,6 @@ InstanceBuilder::InstanceBuilder(

memory_(memory.is_null() ? Handle<JSArrayBuffer>::null()

: memory.ToHandleChecked()),

instance_finalizer_callback_(instance_finalizer_callback) {

- counters_ = counters_shared_.get();

}

// Build an instance, in all of its glory.

@@ -784,9 +771,8 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {

// Record build time into correct bucket, then build instance.

HistogramTimerScope wasm_instantiate_module_time_scope(

- module_->is_wasm()

- ? isolate_->counters()->wasm_instantiate_wasm_module_time()

- : isolate_->counters()->wasm_instantiate_asm_module_time());

+ module_->is_wasm() ? counters()->wasm_instantiate_wasm_module_time()

+ : counters()->wasm_instantiate_asm_module_time());

Factory* factory = isolate_->factory();

//--------------------------------------------------------------------------

@@ -860,7 +846,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {

UNREACHABLE();

}

- RecordStats(code_table, counters_);

+ RecordStats(code_table, counters());

} else {

// There was no owner, so we can reuse the original.

compiled_module_ = original;

@@ -939,8 +925,8 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {

// Set up the memory for the new instance.

//--------------------------------------------------------------------------

uint32_t min_mem_pages = module_->min_mem_pages;

- (module_->is_wasm() ? isolate_->counters()->wasm_wasm_min_mem_pages_count()

- : isolate_->counters()->wasm_asm_min_mem_pages_count())

+ (module_->is_wasm() ? counters()->wasm_wasm_min_mem_pages_count()

+ : counters()->wasm_asm_min_mem_pages_count())

->AddSample(min_mem_pages);

if (!memory_.is_null()) {

@@ -1142,7 +1128,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {

Handle<WasmExportedFunction> startup_fct = WasmExportedFunction::New(

isolate_, instance, MaybeHandle<String>(), start_index,

static_cast<int>(sig->parameter_count()), wrapper_code);

- RecordStats(*startup_code, counters_);

+ RecordStats(*startup_code, counters());

// Call the JS function.

Handle<Object> undefined = factory->undefined_value();

MaybeHandle<Object> retval =

@@ -1339,7 +1325,7 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,

return -1;

}

code_table->set(num_imported_functions, *import_wrapper);

- RecordStats(*import_wrapper, counters_);

+ RecordStats(*import_wrapper, counters());

num_imported_functions++;

break;

}

@@ -1924,7 +1910,6 @@ AsyncCompileJob::AsyncCompileJob(Isolate* isolate,

context_ = Handle<Context>(*context);

module_promise_ = Handle<JSPromise>(*promise);

deferred_handles_.push_back(deferred.Detach());

- counters_ = counters_shared_.get();

}

void AsyncCompileJob::Start() {

@@ -2049,10 +2034,9 @@ class AsyncCompileJob::DecodeModule : public AsyncCompileJob::CompileStep {

DisallowHeapAllocation no_allocation;

// Decode the module bytes.

TRACE_COMPILE("(1) Decoding module...\n");

- constexpr bool is_sync = true;

- result = DecodeWasmModule(job_->isolate_, job_->wire_bytes_.start(),

- job_->wire_bytes_.end(), false, kWasmOrigin,

- !is_sync);

+ result = AsyncDecodeWasmModule(job_->isolate_, job_->wire_bytes_.start(),

+ job_->wire_bytes_.end(), false,

+ kWasmOrigin, job_->counters_shared_);

Mircea Trofin 2017/06/22 18:42:24 why not the accessor call (job_->counters()) ?

kschimpf 2017/06/22 20:38:31 Because the API expected a shared_ptr<Counters>. H

}

if (result.failed()) {

// Decoding failure; reject the promise and clean up.

@@ -2138,7 +2122,7 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep {

job_->temp_instance_->function_code[i] = illegal_builtin;

}

- job_->isolate_->counters()->wasm_functions_per_wasm_module()->AddSample(

+ job_->counters()->wasm_functions_per_wasm_module()->AddSample(

static_cast<int>(module_->functions.size()));

// Transfer ownership of the {WasmModule} to the {ModuleCompiler}, but

@@ -2292,7 +2276,7 @@ class AsyncCompileJob::FinishCompile : public CompileStep {

for (size_t i = FLAG_skip_compiling_wasm_funcs;

i < job_->temp_instance_->function_code.size(); ++i) {

Code* code = Code::cast(job_->code_table_->get(static_cast<int>(i)));

- RecordStats(code, job_->counters_);

+ RecordStats(code, job_->counters());

}

// Create heap objects for script and module bytes to be stored in the

@@ -2368,7 +2352,7 @@ class AsyncCompileJob::CompileWrappers : public CompileStep {

int export_index =

static_cast<int>(module->functions.size() + func_index);

job_->code_table_->set(export_index, *wrapper_code);

- RecordStats(*wrapper_code, job_->counters_);

+ RecordStats(*wrapper_code, job_->counters());

func_index++;

}

« src/wasm/module-compiler.h ('K') | « src/wasm/module-compiler.h ('k') | src/wasm/module-decoder.h » ('j') | no next file with comments »