Commit fd52adcb authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[wasm] Use modification scopes on module level and not function level

During WebAssembly compilation and instantiation we entered a
{CodeSpaceMemoryModificationScope} several times per function. This
introduced significant overhead, see the referenced bug. With this CL
we enter the {CodeSpaceMemoryModificationScope} on a per-module
granularity and not on a function granularity. We enter now the
following scopes:

* one scope for the whole synchronous compilation;
* one scope for each finishing step in asynchronous compilation (each
    step finishes multiple functions);
* one scope for module instantiation, without the execution of the
  start function.

Locally these changes reduced the overhead significantly.

R=mstarzinger@chromium.org, titzer@chromium.org
CC=clemensh@chromium.org

Bug: chromium:787731
Change-Id: I5c5694544a97f4c1e5a2a29da9a005d0ca7616bd
Reviewed-on: https://chromium-review.googlesource.com/787851Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49618}
parent 6381c541
......@@ -9,6 +9,7 @@
#include "src/api.h"
#include "src/asmjs/asm-js.h"
#include "src/assembler-inl.h"
#include "src/base/optional.h"
#include "src/base/template-utils.h"
#include "src/base/utils/random-number-generator.h"
#include "src/code-stubs.h"
......@@ -1444,6 +1445,10 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
TimedHistogramScope wasm_compile_module_time_scope(
module_->is_wasm() ? counters()->wasm_compile_wasm_module_time()
: counters()->wasm_compile_asm_module_time());
// TODO(6792): No longer needed once WebAssembly code is off heap. Use
// base::Optional to be able to close the scope before notifying the debugger.
base::Optional<CodeSpaceMemoryModificationScope> modification_scope(
base::in_place_t(), isolate_->heap());
// The {module> parameter is passed in to transfer ownership of the WasmModule
// to this function. The WasmModule itself existed already as an instance
// variable of the ModuleCompiler. We check here that the parameter and the
......@@ -1563,6 +1568,9 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
// If we created a wasm script, finish it now and make it public to the
// debugger.
if (asm_js_script.is_null()) {
// Close the CodeSpaceMemoryModificationScope before calling into the
// debugger.
modification_scope.reset();
script->set_wasm_compiled_module(*compiled_module);
isolate_->debug()->OnAfterCompile(script);
}
......@@ -1590,6 +1598,11 @@ InstanceBuilder::InstanceBuilder(
// Build an instance, in all of its glory.
MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
// TODO(6792): No longer needed once WebAssembly code is off heap.
// Use base::Optional to be able to close the scope before executing the start
// function.
base::Optional<CodeSpaceMemoryModificationScope> modification_scope(
base::in_place_t(), isolate_->heap());
// 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()) {
......@@ -1952,6 +1965,8 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
RecordStats(*startup_code, counters());
// Call the JS function.
Handle<Object> undefined = factory->undefined_value();
// Close the CodeSpaceMemoryModificationScope to execute the start function.
modification_scope.reset();
{
// We're OK with JS execution here. The instance is fully setup.
AllowJavascriptExecution allow_js(isolate_);
......@@ -3159,6 +3174,11 @@ class AsyncCompileJob::ExecuteAndFinishCompilationUnits : public CompileStep {
}
void RunInForeground() override {
// TODO(6792): No longer needed once WebAssembly code is off heap.
// Use base::Optional to be able to close the scope before we resolve or
// reject the promise.
base::Optional<CodeSpaceMemoryModificationScope> modification_scope(
base::in_place_t(), job_->isolate_->heap());
TRACE_COMPILE("(4a) Finishing compilation units...\n");
if (failed_) {
// The job failed already, no need to do more work.
......@@ -3210,6 +3230,10 @@ class AsyncCompileJob::ExecuteAndFinishCompilationUnits : public CompileStep {
if (thrower.error()) {
// Make sure all compilation tasks stopped running.
job_->background_task_manager_.CancelAndWait();
// Close the CodeSpaceMemoryModificationScope before we reject the promise
// in AsyncCompileFailed. Promise::Reject calls directly into JavaScript.
modification_scope.reset();
return job_->AsyncCompileFailed(thrower);
}
if (job_->outstanding_units_ == 0) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment