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: Make explicit when async_counters is needed. 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..2e4173ba326413f53af10c680f55ca69f185afff 100644

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

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

@@ -85,7 +85,7 @@ ModuleCompiler::ModuleCompiler(Isolate* isolate,

std::unique_ptr<WasmModule> module, bool is_sync)

: isolate_(isolate),

module_(std::move(module)),

- counters_shared_(isolate->counters_shared()),

+ async_counters_(isolate->async_counters()),

is_sync_(is_sync),

executed_units_(

isolate->random_number_generator(),

@@ -96,9 +96,7 @@ ModuleCompiler::ModuleCompiler(Isolate* isolate,

num_background_tasks_(

Min(static_cast<size_t>(FLAG_wasm_num_compilation_tasks),

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

- stopped_compilation_tasks_(num_background_tasks_) {

- counters_ = counters_shared_.get();

+ stopped_compilation_tasks_(num_background_tasks_) {}

bool ModuleCompiler::GetNextUncompiledFunctionId(size_t* index) {

DCHECK_NOT_NULL(index);

@@ -184,10 +182,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 +361,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()

+ : 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 +602,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 +637,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 +703,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++;

}

@@ -762,15 +749,13 @@ InstanceBuilder::InstanceBuilder(

WeakCallbackInfo<void>::Callback instance_finalizer_callback)

: isolate_(isolate),

module_(module_object->compiled_module()->module()),

- counters_shared_(isolate->counters_shared()),

+ async_counters_(isolate->async_counters()),

thrower_(thrower),

module_object_(module_object),

ffi_(ffi.is_null() ? Handle<JSReceiver>::null() : ffi.ToHandleChecked()),

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

: memory.ToHandleChecked()),

- instance_finalizer_callback_(instance_finalizer_callback) {

- counters_ = counters_shared_.get();

+ instance_finalizer_callback_(instance_finalizer_callback) {}

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

MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {

@@ -784,9 +769,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 +844,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 +923,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 +1126,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 +1323,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;

}

@@ -1916,7 +1900,7 @@ AsyncCompileJob::AsyncCompileJob(Isolate* isolate,

size_t length, Handle<Context> context,

Handle<JSPromise> promise)

: isolate_(isolate),

- counters_shared_(isolate->counters_shared()),

+ async_counters_(isolate->async_counters()),

bytes_copy_(std::move(bytes_copy)),

wire_bytes_(bytes_copy_.get(), bytes_copy_.get() + length) {

// The handles for the context and promise must be deferred.

@@ -1924,7 +1908,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 +2032,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());

}

if (result.failed()) {

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

@@ -2138,7 +2120,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 +2274,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 +2350,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++;

}

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