Commit e6414f6e authored by Paolo Severini's avatar Paolo Severini Committed by Commit Bot

[wasm] Faster wasm-to-js calls with arguments mismatch

Currently WebAssembly always goes through the ArgumentsAdaptorTrampoline
builtin for wasm-to-js calls as soon as there's a mismatch between the
actual number of arguments and the expected number of arguments.

This can be made faster in cases where:
1. the callee has "don't adapt arguments" set, which is often the case
for builtins, or
2. the callee has "skip adapt arguments" set, which is often the case
for strict mode functions.

TurboFan already supports this for JS calls:
https://chromium-review.googlesource.com/c/1482735;
explainer document:
http://bit.ly/v8-faster-calls-with-arguments-mismatch.

Even though it is probably not as common to have arity mismatches in
Wasm->JS calls as it is in JS->JS calls, this still seems a worthwhile
optimization to do.

This CL ports the TurboFan fix to WebAssembly. In particular, the CL
introduces a new WasmImportCallKind (kJSFunctionArityMismatchSkipAdaptor)
for the case where the call to  Builtins_ArgumentsAdaptorTrampoline
can be skipped, and modifies WasmImportWrapperCache::CacheKey to also
consider the arity of the imported JS function.

A micro-benchmark for this change can be found here:
- https://gist.github.com/paolosevMSFT/72c67591170d6163f67c9b03a7e12525#file-adapter-cc
- https://gist.github.com/paolosevMSFT/72c67591170d6163f67c9b03a7e12525#file-adapter_test-js

With this benchmark, we can save a 40% overhead of
Builtins_ArgumentsAdaptorTrampoline for calls that pass too many
arguments, while the savings for calls that pass too few arguments are
less impressive:

                            Before     After
callProperApplication:      563 ms     566 ms
callOverApplication1:       972 ms     562 ms
callOverApplication2:       962 ms     562 ms
callUnderApplication:       949 ms     890 ms


Bug: v8:8909
Change-Id: Id51764e7c422d00ecc4a48704323e11bdca9377f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2317061
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69110}
parent 97088422
......@@ -6213,7 +6213,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
global_proxy);
}
bool BuildWasmImportCallWrapper(WasmImportCallKind kind) {
bool BuildWasmImportCallWrapper(WasmImportCallKind kind, int expected_arity) {
int wasm_count = static_cast<int>(sig_->parameter_count());
// Build the start and the parameter nodes.
......@@ -6327,6 +6327,47 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
break;
}
// =======================================================================
// === JS Functions without arguments adapter ============================
// =======================================================================
case WasmImportCallKind::kJSFunctionArityMismatchSkipAdaptor: {
base::SmallVector<Node*, 16> args(expected_arity + 7);
int pos = 0;
Node* function_context =
gasm_->Load(MachineType::TaggedPointer(), callable_node,
wasm::ObjectAccess::ContextOffsetInTaggedJSFunction());
args[pos++] = callable_node; // target callable.
// Determine receiver at runtime.
args[pos++] =
BuildReceiverNode(callable_node, native_context, undefined_node);
auto call_descriptor = Linkage::GetJSCallDescriptor(
graph()->zone(), false, expected_arity + 1,
CallDescriptor::kNoFlags);
// Convert wasm numbers to JS values.
if (expected_arity <= wasm_count) {
pos = AddArgumentNodes(VectorOf(args), pos, expected_arity, sig_);
} else {
pos = AddArgumentNodes(VectorOf(args), pos, wasm_count, sig_);
for (int i = wasm_count; i < expected_arity; ++i) {
args[pos++] = undefined_node;
}
}
args[pos++] = undefined_node; // new target
args[pos++] =
mcgraph()->Int32Constant(expected_arity); // argument count
args[pos++] = function_context;
args[pos++] = effect();
args[pos++] = control();
DCHECK_EQ(pos, args.size());
call = graph()->NewNode(mcgraph()->common()->Call(call_descriptor), pos,
args.begin());
break;
}
// =======================================================================
// === General case of unknown callable ==================================
// =======================================================================
case WasmImportCallKind::kUseCallBuiltin: {
......@@ -6765,7 +6806,8 @@ std::pair<WasmImportCallKind, Handle<JSReceiver>> ResolveWasmImportCall(
// For JavaScript calls, determine whether the target has an arity match.
if (callable->IsJSFunction()) {
Handle<JSFunction> function = Handle<JSFunction>::cast(callable);
SharedFunctionInfo shared = function->shared();
Handle<SharedFunctionInfo> shared(function->shared(),
function->GetIsolate());
// Check for math intrinsics.
#define COMPARE_SIG_FOR_BUILTIN(name) \
......@@ -6788,8 +6830,8 @@ std::pair<WasmImportCallKind, Handle<JSReceiver>> ResolveWasmImportCall(
COMPARE_SIG_FOR_BUILTIN(F32##name); \
break;
if (FLAG_wasm_math_intrinsics && shared.HasBuiltinId()) {
switch (shared.builtin_id()) {
if (FLAG_wasm_math_intrinsics && shared->HasBuiltinId()) {
switch (shared->builtin_id()) {
COMPARE_SIG_FOR_BUILTIN_F64(Acos);
COMPARE_SIG_FOR_BUILTIN_F64(Asin);
COMPARE_SIG_FOR_BUILTIN_F64(Atan);
......@@ -6818,15 +6860,29 @@ std::pair<WasmImportCallKind, Handle<JSReceiver>> ResolveWasmImportCall(
#undef COMPARE_SIG_FOR_BUILTIN_F64
#undef COMPARE_SIG_FOR_BUILTIN_F32_F64
if (IsClassConstructor(shared.kind())) {
if (IsClassConstructor(shared->kind())) {
// Class constructor will throw anyway.
return std::make_pair(WasmImportCallKind::kUseCallBuiltin, callable);
}
if (shared.internal_formal_parameter_count() ==
if (shared->internal_formal_parameter_count() ==
expected_sig->parameter_count()) {
return std::make_pair(WasmImportCallKind::kJSFunctionArityMatch,
callable);
}
// If function isn't compiled, compile it now.
IsCompiledScope is_compiled_scope(
shared->is_compiled_scope(callable->GetIsolate()));
if (!is_compiled_scope.is_compiled()) {
Compiler::Compile(function, Compiler::CLEAR_EXCEPTION,
&is_compiled_scope);
}
if (shared->is_safe_to_skip_arguments_adaptor()) {
return std::make_pair(
WasmImportCallKind::kJSFunctionArityMismatchSkipAdaptor, callable);
}
return std::make_pair(WasmImportCallKind::kJSFunctionArityMismatch,
callable);
}
......@@ -6941,7 +6997,7 @@ wasm::WasmCompilationResult CompileWasmMathIntrinsic(
wasm::WasmCompilationResult CompileWasmImportCallWrapper(
wasm::WasmEngine* wasm_engine, wasm::CompilationEnv* env,
WasmImportCallKind kind, const wasm::FunctionSig* sig,
bool source_positions) {
bool source_positions, int expected_arity) {
DCHECK_NE(WasmImportCallKind::kLinkError, kind);
DCHECK_NE(WasmImportCallKind::kWasmToWasm, kind);
......@@ -6972,7 +7028,7 @@ wasm::WasmCompilationResult CompileWasmImportCallWrapper(
WasmWrapperGraphBuilder builder(&zone, mcgraph, sig, source_position_table,
StubCallMode::kCallWasmRuntimeStub,
env->enabled_features);
builder.BuildWasmImportCallWrapper(kind);
builder.BuildWasmImportCallWrapper(kind, expected_arity);
// Build a name in the form "wasm-to-js-<kind>-<signature>".
constexpr size_t kMaxNameLen = 128;
......
......@@ -58,12 +58,15 @@ wasm::WasmCompilationResult ExecuteTurbofanWasmCompilation(
// type of the target function/callable and whether the signature matches the
// argument arity.
enum class WasmImportCallKind : uint8_t {
kLinkError, // static Wasm->Wasm type error
kRuntimeTypeError, // runtime Wasm->JS type error
kWasmToCapi, // fast Wasm->C-API call
kWasmToWasm, // fast Wasm->Wasm call
kJSFunctionArityMatch, // fast Wasm->JS call
kJSFunctionArityMismatch, // Wasm->JS, needs adapter frame
kLinkError, // static Wasm->Wasm type error
kRuntimeTypeError, // runtime Wasm->JS type error
kWasmToCapi, // fast Wasm->C-API call
kWasmToWasm, // fast Wasm->Wasm call
kJSFunctionArityMatch, // fast Wasm->JS call
kJSFunctionArityMismatch, // Wasm->JS, needs adapter frame
kJSFunctionArityMismatchSkipAdaptor, // Wasm->JS, arity mismatch calling
// strict mode function where we don't
// need the ArgumentsAdaptorTrampoline.
// Math functions imported from JavaScript that are intrinsified
kFirstMathIntrinsic,
kF64Acos = kFirstMathIntrinsic,
......@@ -108,7 +111,7 @@ ResolveWasmImportCall(Handle<JSReceiver> callable, const wasm::FunctionSig* sig,
// Compiles an import call wrapper, which allows Wasm to call imports.
V8_EXPORT_PRIVATE wasm::WasmCompilationResult CompileWasmImportCallWrapper(
wasm::WasmEngine*, wasm::CompilationEnv* env, WasmImportCallKind,
const wasm::FunctionSig*, bool source_positions);
const wasm::FunctionSig*, bool source_positions, int expected_arity);
// Compiles a host call wrapper, which allows Wasm to call host functions.
wasm::WasmCode* CompileWasmCapiCallWrapper(wasm::WasmEngine*,
......
......@@ -150,7 +150,8 @@ WasmCompilationResult WasmCompilationUnit::ExecuteImportWrapperCompilation(
auto kind = compiler::kDefaultImportCallKind;
bool source_positions = is_asmjs_module(env->module);
WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
engine, env, kind, sig, source_positions);
engine, env, kind, sig, source_positions,
static_cast<int>(sig->parameter_count()));
return result;
}
......
......@@ -1145,8 +1145,9 @@ bool ExecuteCompilationUnits(
if (func_index < num_imported_functions) {
const FunctionSig* sig =
native_module->module()->functions[func_index].sig;
WasmImportWrapperCache::CacheKey key(compiler::kDefaultImportCallKind,
sig);
WasmImportWrapperCache::CacheKey key(
compiler::kDefaultImportCallKind, sig,
static_cast<int>(sig->parameter_count()));
// If two imported functions have the same key, only one of them should
// have been added as a compilation unit. So it is always the first time
// we compile a wrapper for this key here.
......@@ -1242,7 +1243,9 @@ int AddImportWrapperUnits(NativeModule* native_module,
if (!IsJSCompatibleSignature(sig, native_module->enabled_features())) {
continue;
}
WasmImportWrapperCache::CacheKey key(compiler::kDefaultImportCallKind, sig);
WasmImportWrapperCache::CacheKey key(
compiler::kDefaultImportCallKind, sig,
static_cast<int>(sig->parameter_count()));
auto it = keys.insert(key);
if (it.second) {
// Ensure that all keys exist in the cache, so that we can populate the
......@@ -3053,18 +3056,19 @@ void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module,
WasmCode* CompileImportWrapper(
WasmEngine* wasm_engine, NativeModule* native_module, Counters* counters,
compiler::WasmImportCallKind kind, const FunctionSig* sig,
int expected_arity,
WasmImportWrapperCache::ModificationScope* cache_scope) {
// Entry should exist, so that we don't insert a new one and invalidate
// other threads' iterators/references, but it should not have been compiled
// yet.
WasmImportWrapperCache::CacheKey key(kind, sig);
WasmImportWrapperCache::CacheKey key(kind, sig, expected_arity);
DCHECK_NULL((*cache_scope)[key]);
bool source_positions = is_asmjs_module(native_module->module());
// Keep the {WasmCode} alive until we explicitly call {IncRef}.
WasmCodeRefScope code_ref_scope;
CompilationEnv env = native_module->CreateCompilationEnv();
WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
wasm_engine, &env, kind, sig, source_positions);
wasm_engine, &env, kind, sig, source_positions, expected_arity);
std::unique_ptr<WasmCode> wasm_code = native_module->AddCode(
result.func_index, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
......
......@@ -58,7 +58,7 @@ V8_EXPORT_PRIVATE
WasmCode* CompileImportWrapper(
WasmEngine* wasm_engine, NativeModule* native_module, Counters* counters,
compiler::WasmImportCallKind kind, const FunctionSig* sig,
WasmImportWrapperCache::ModificationScope* cache_scope);
int expected_arity, WasmImportWrapperCache::ModificationScope* cache_scope);
// Triggered by the WasmCompileLazy builtin. The return value indicates whether
// compilation was successful. Lazy compilation can fail only if validation is
......
......@@ -73,8 +73,8 @@ class CompileImportWrapperTask final : public CancelableTask {
void RunInternal() override {
while (base::Optional<WasmImportWrapperCache::CacheKey> key =
queue_->pop()) {
CompileImportWrapper(engine_, native_module_, counters_, key->first,
key->second, cache_scope_);
CompileImportWrapper(engine_, native_module_, counters_, key->kind,
key->signature, key->expected_arity, cache_scope_);
}
}
......@@ -1006,9 +1006,18 @@ bool InstanceBuilder::ProcessImportedFunction(
}
default: {
// The imported function is a callable.
int expected_arity = static_cast<int>(expected_sig->parameter_count());
if (kind ==
compiler::WasmImportCallKind::kJSFunctionArityMismatchSkipAdaptor) {
Handle<JSFunction> function = Handle<JSFunction>::cast(js_receiver);
SharedFunctionInfo shared = function->shared();
expected_arity = shared.internal_formal_parameter_count();
}
NativeModule* native_module = instance->module_object().native_module();
WasmCode* wasm_code =
native_module->import_wrapper_cache()->Get(kind, expected_sig);
WasmCode* wasm_code = native_module->import_wrapper_cache()->Get(
kind, expected_sig, expected_arity);
DCHECK_NOT_NULL(wasm_code);
ImportedFunctionEntry entry(instance, func_index);
if (wasm_code->kind() == WasmCode::kWasmToJsWrapper) {
......@@ -1346,7 +1355,16 @@ void InstanceBuilder::CompileImportWrappers(
kind == compiler::WasmImportCallKind::kWasmToCapi) {
continue;
}
WasmImportWrapperCache::CacheKey key(kind, sig);
int expected_arity = static_cast<int>(sig->parameter_count());
if (resolved.first ==
compiler::WasmImportCallKind::kJSFunctionArityMismatchSkipAdaptor) {
Handle<JSFunction> function = Handle<JSFunction>::cast(resolved.second);
SharedFunctionInfo shared = function->shared();
expected_arity = shared.internal_formal_parameter_count();
}
WasmImportWrapperCache::CacheKey key(kind, sig, expected_arity);
if (cache_scope[key] != nullptr) {
// Cache entry already exists, no need to compile it again.
continue;
......@@ -1367,8 +1385,8 @@ void InstanceBuilder::CompileImportWrappers(
while (base::Optional<WasmImportWrapperCache::CacheKey> key =
import_wrapper_queue.pop()) {
CompileImportWrapper(isolate_->wasm_engine(), native_module,
isolate_->counters(), key->first, key->second,
&cache_scope);
isolate_->counters(), key->kind, key->signature,
key->expected_arity, &cache_scope);
}
task_manager.CancelAndWait();
}
......
......@@ -24,9 +24,11 @@ WasmCode*& WasmImportWrapperCache::operator[](
}
WasmCode* WasmImportWrapperCache::Get(compiler::WasmImportCallKind kind,
const FunctionSig* sig) const {
const FunctionSig* sig,
int expected_arity) const {
base::MutexGuard lock(&mutex_);
auto it = entry_map_.find({kind, sig});
auto it = entry_map_.find({kind, sig, expected_arity});
DCHECK(it != entry_map_.end());
return it->second;
}
......
......@@ -23,12 +23,30 @@ using FunctionSig = Signature<ValueType>;
// Implements a cache for import wrappers.
class WasmImportWrapperCache {
public:
using CacheKey = std::pair<compiler::WasmImportCallKind, const FunctionSig*>;
struct CacheKey {
CacheKey(const compiler::WasmImportCallKind& _kind, const FunctionSig* _sig,
int _expected_arity)
: kind(_kind),
signature(_sig),
expected_arity(_expected_arity == kDontAdaptArgumentsSentinel
? 0
: _expected_arity) {}
bool operator==(const CacheKey& rhs) const {
return kind == rhs.kind && signature == rhs.signature &&
expected_arity == rhs.expected_arity;
}
compiler::WasmImportCallKind kind;
const FunctionSig* signature;
int expected_arity;
};
class CacheKeyHash {
public:
size_t operator()(const CacheKey& key) const {
return base::hash_combine(static_cast<uint8_t>(key.first), *key.second);
return base::hash_combine(static_cast<uint8_t>(key.kind), key.signature,
key.expected_arity);
}
};
......@@ -51,7 +69,8 @@ class WasmImportWrapperCache {
// Thread-safe. Assumes the key exists in the map.
V8_EXPORT_PRIVATE WasmCode* Get(compiler::WasmImportCallKind kind,
const FunctionSig* sig) const;
const FunctionSig* sig,
int expected_arity) const;
~WasmImportWrapperCache();
......
......@@ -1447,8 +1447,10 @@ void WasmInstanceObject::ImportWasmJSFunctionIntoTable(
callable = resolved.second; // Update to ultimate target.
DCHECK_NE(compiler::WasmImportCallKind::kLinkError, kind);
wasm::CompilationEnv env = native_module->CreateCompilationEnv();
SharedFunctionInfo shared = js_function->shared();
wasm::WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
isolate->wasm_engine(), &env, kind, sig, false);
isolate->wasm_engine(), &env, kind, sig, false,
shared.internal_formal_parameter_count());
std::unique_ptr<wasm::WasmCode> wasm_code = native_module->AddCode(
result.func_index, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
......
......@@ -36,15 +36,17 @@ TEST(CacheHit) {
module->import_wrapper_cache());
auto kind = compiler::WasmImportCallKind::kJSFunctionArityMatch;
auto sig = sigs.i_i();
int expected_arity = static_cast<int>(sig->parameter_count());
WasmCode* c1 =
CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sigs.i_i(), &cache_scope);
WasmCode* c1 = CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sig,
expected_arity, &cache_scope);
CHECK_NOT_NULL(c1);
CHECK_EQ(WasmCode::Kind::kWasmToJsWrapper, c1->kind());
WasmCode* c2 = cache_scope[{kind, sigs.i_i()}];
WasmCode* c2 = cache_scope[{kind, sig, expected_arity}];
CHECK_NOT_NULL(c2);
CHECK_EQ(c1, c2);
......@@ -59,15 +61,19 @@ TEST(CacheMissSig) {
module->import_wrapper_cache());
auto kind = compiler::WasmImportCallKind::kJSFunctionArityMatch;
auto sig1 = sigs.i_i();
int expected_arity1 = static_cast<int>(sig1->parameter_count());
auto sig2 = sigs.i_ii();
int expected_arity2 = static_cast<int>(sig2->parameter_count());
WasmCode* c1 =
CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sigs.i_i(), &cache_scope);
WasmCode* c1 = CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sig1,
expected_arity1, &cache_scope);
CHECK_NOT_NULL(c1);
CHECK_EQ(WasmCode::Kind::kWasmToJsWrapper, c1->kind());
WasmCode* c2 = cache_scope[{kind, sigs.i_ii()}];
WasmCode* c2 = cache_scope[{kind, sig2, expected_arity2}];
CHECK_NULL(c2);
}
......@@ -82,15 +88,17 @@ TEST(CacheMissKind) {
auto kind1 = compiler::WasmImportCallKind::kJSFunctionArityMatch;
auto kind2 = compiler::WasmImportCallKind::kJSFunctionArityMismatch;
auto sig = sigs.i_i();
int expected_arity = static_cast<int>(sig->parameter_count());
WasmCode* c1 = CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind1, sigs.i_i(),
&cache_scope);
isolate->counters(), kind1, sig,
expected_arity, &cache_scope);
CHECK_NOT_NULL(c1);
CHECK_EQ(WasmCode::Kind::kWasmToJsWrapper, c1->kind());
WasmCode* c2 = cache_scope[{kind2, sigs.i_i()}];
WasmCode* c2 = cache_scope[{kind2, sig, expected_arity}];
CHECK_NULL(c2);
}
......@@ -104,30 +112,34 @@ TEST(CacheHitMissSig) {
module->import_wrapper_cache());
auto kind = compiler::WasmImportCallKind::kJSFunctionArityMatch;
auto sig1 = sigs.i_i();
int expected_arity1 = static_cast<int>(sig1->parameter_count());
auto sig2 = sigs.i_ii();
int expected_arity2 = static_cast<int>(sig2->parameter_count());
WasmCode* c1 =
CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sigs.i_i(), &cache_scope);
WasmCode* c1 = CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sig1,
expected_arity1, &cache_scope);
CHECK_NOT_NULL(c1);
CHECK_EQ(WasmCode::Kind::kWasmToJsWrapper, c1->kind());
WasmCode* c2 = cache_scope[{kind, sigs.i_ii()}];
WasmCode* c2 = cache_scope[{kind, sig2, expected_arity2}];
CHECK_NULL(c2);
c2 = CompileImportWrapper(isolate->wasm_engine(), module.get(),
isolate->counters(), kind, sigs.i_ii(),
isolate->counters(), kind, sig2, expected_arity2,
&cache_scope);
CHECK_NE(c1, c2);
WasmCode* c3 = cache_scope[{kind, sigs.i_i()}];
WasmCode* c3 = cache_scope[{kind, sig1, expected_arity1}];
CHECK_NOT_NULL(c3);
CHECK_EQ(c1, c3);
WasmCode* c4 = cache_scope[{kind, sigs.i_ii()}];
WasmCode* c4 = cache_scope[{kind, sig2, expected_arity2}];
CHECK_NOT_NULL(c4);
CHECK_EQ(c2, c4);
......
......@@ -54,12 +54,15 @@ TestingModuleBuilder::TestingModuleBuilder(
Handle<JSReceiver> callable = resolved.second;
WasmImportWrapperCache::ModificationScope cache_scope(
native_module_->import_wrapper_cache());
WasmImportWrapperCache::CacheKey key(kind, maybe_import->sig);
WasmImportWrapperCache::CacheKey key(
kind, maybe_import->sig,
static_cast<int>(maybe_import->sig->parameter_count()));
auto import_wrapper = cache_scope[key];
if (import_wrapper == nullptr) {
import_wrapper = CompileImportWrapper(
isolate_->wasm_engine(), native_module_, isolate_->counters(), kind,
maybe_import->sig, &cache_scope);
maybe_import->sig,
static_cast<int>(maybe_import->sig->parameter_count()), &cache_scope);
}
ImportedFunctionEntry(instance_object_, maybe_import_index)
......
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