From f3a70ff9e62d26318a6e9a9a84b9a38dcfce85f7 Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Fri, 12 Aug 2022 17:21:56 +0000 Subject: [PATCH] Bug 1782770 - wasm: Don't make extra copy of FuncType for debugging. r=yury Now that we transfer all type definitions to Metadata, we can remove the special debugging case where we would transfer all function types. Instead, we can just transfer the funcTypeIndex and find the function type in Metadata. Differential Revision: https://phabricator.services.mozilla.com/D153500 --- js/src/wasm/WasmCode.h | 16 ++++++---------- js/src/wasm/WasmDebug.cpp | 7 ++++--- js/src/wasm/WasmDebug.h | 6 ------ js/src/wasm/WasmDebugFrame.cpp | 4 +++- js/src/wasm/WasmGenerator.cpp | 14 ++------------ js/src/wasm/WasmInstance.cpp | 2 +- js/src/wasm/WasmSerialize.cpp | 6 ++---- js/src/wasm/WasmStubs.cpp | 1 + 8 files changed, 19 insertions(+), 37 deletions(-) diff --git a/js/src/wasm/WasmCode.h b/js/src/wasm/WasmCode.h index d92eae2fb4f4..618f452c62d9 100644 --- a/js/src/wasm/WasmCode.h +++ b/js/src/wasm/WasmCode.h @@ -372,8 +372,6 @@ struct MetadataCacheablePod { WASM_DECLARE_CACHEABLE_POD(MetadataCacheablePod) typedef uint8_t ModuleHash[8]; -using FuncArgTypesVector = Vector; -using FuncReturnTypesVector = Vector; struct Metadata : public ShareableBase, public MetadataCacheablePod { TypeDefVector types; @@ -393,8 +391,7 @@ struct Metadata : public ShareableBase, public MetadataCacheablePod { // Debug-enabled code is not serialized. bool debugEnabled; - FuncArgTypesVector debugFuncArgTypes; - FuncReturnTypesVector debugFuncReturnTypes; + Uint32Vector debugFuncTypeIndices; ModuleHash debugHash; explicit Metadata(ModuleKind kind = ModuleKind::Wasm) @@ -416,12 +413,11 @@ struct Metadata : public ShareableBase, public MetadataCacheablePod { return types[funcExport.typeIndex()].funcType(); } - // Invariant: The result of getFuncResultType can only be used as long as - // Metadata is live, because the returned ResultType may encode a pointer to - // debugFuncReturnTypes. - ResultType getFuncResultType(uint32_t funcIndex) const { - return ResultType::Vector(debugFuncReturnTypes[funcIndex]); - }; + size_t debugNumFuncs() const { return debugFuncTypeIndices.length(); } + const FuncType& debugFuncType(uint32_t funcIndex) const { + MOZ_ASSERT(debugEnabled); + return types[debugFuncTypeIndices[funcIndex]].funcType(); + } // AsmJSMetadata derives Metadata iff isAsmJS(). Mostly this distinction is // encapsulated within AsmJS.cpp, but the additional virtual functions allow diff --git a/js/src/wasm/WasmDebug.cpp b/js/src/wasm/WasmDebug.cpp index 60fc8a366af9..9185e62deb13 100644 --- a/js/src/wasm/WasmDebug.cpp +++ b/js/src/wasm/WasmDebug.cpp @@ -318,7 +318,7 @@ void DebugState::adjustEnterAndLeaveFrameTrapsState(JSContext* cx, } MOZ_RELEASE_ASSERT(&instance->metadata() == &metadata()); - uint32_t numFuncs = metadata().debugFuncReturnTypes.length(); + uint32_t numFuncs = metadata().debugNumFuncs(); if (enabled) { MOZ_ASSERT(enterAndLeaveFrameTrapsCounter_ > 0); for (uint32_t funcIdx = 0; funcIdx < numFuncs; funcIdx++) { @@ -372,8 +372,9 @@ void DebugState::ensureEnterFrameTrapsState(JSContext* cx, Instance* instance, bool DebugState::debugGetLocalTypes(uint32_t funcIndex, ValTypeVector* locals, size_t* argsLength, StackResults* stackResults) { - const ValTypeVector& args = metadata().debugFuncArgTypes[funcIndex]; - const ValTypeVector& results = metadata().debugFuncReturnTypes[funcIndex]; + const FuncType& funcType = metadata().debugFuncType(funcIndex); + const ValTypeVector& args = funcType.args(); + const ValTypeVector& results = funcType.results(); ResultType resultType(ResultType::Vector(results)); *argsLength = args.length(); *stackResults = ABIResultIter::HasStackResults(resultType) diff --git a/js/src/wasm/WasmDebug.h b/js/src/wasm/WasmDebug.h index 41f15354a893..35c8a838ed4a 100644 --- a/js/src/wasm/WasmDebug.h +++ b/js/src/wasm/WasmDebug.h @@ -150,12 +150,6 @@ class DebugState { ValTypeVector* locals, size_t* argsLength, StackResults* stackResults); - // Invariant: the result of getDebugResultType can only be used as long as - // code_->metadata() is live. See MetaData::getFuncResultType for more - // information. - ResultType debugGetResultType(uint32_t funcIndex) const { - return metadata().getFuncResultType(funcIndex); - } [[nodiscard]] bool getGlobal(Instance& instance, uint32_t globalIndex, MutableHandleValue vp); diff --git a/js/src/wasm/WasmDebugFrame.cpp b/js/src/wasm/WasmDebugFrame.cpp index 4dcef877b9a0..2c93e750bc95 100644 --- a/js/src/wasm/WasmDebugFrame.cpp +++ b/js/src/wasm/WasmDebugFrame.cpp @@ -26,6 +26,7 @@ #include "wasm/WasmStubs.h" #include "vm/NativeObject-inl.h" +#include "wasm/WasmInstance-inl.h" using namespace js; using namespace js::jit; @@ -134,7 +135,8 @@ bool DebugFrame::updateReturnJSValue(JSContext* cx) { MutableHandleValue::fromMarkedLocation(&cachedReturnJSValue_); rval.setUndefined(); flags_.hasCachedReturnJSValue = true; - ResultType resultType = instance()->debug().debugGetResultType(funcIndex()); + ResultType resultType = ResultType::Vector( + instance()->metadata().debugFuncType(funcIndex()).results()); Maybe stackResultsLoc; if (ABIResultIter::HasStackResults(resultType)) { stackResultsLoc = Some(static_cast(stackResultsPointer_)); diff --git a/js/src/wasm/WasmGenerator.cpp b/js/src/wasm/WasmGenerator.cpp index 2fee5652b0c0..3c2cc80e806b 100644 --- a/js/src/wasm/WasmGenerator.cpp +++ b/js/src/wasm/WasmGenerator.cpp @@ -1074,21 +1074,11 @@ SharedMetadata ModuleGenerator::finishMetadata(const Bytes& bytecode) { metadata_->debugEnabled = true; const size_t numFuncs = moduleEnv_->funcs.length(); - if (!metadata_->debugFuncArgTypes.resize(numFuncs)) { - return nullptr; - } - if (!metadata_->debugFuncReturnTypes.resize(numFuncs)) { + if (!metadata_->debugFuncTypeIndices.resize(numFuncs)) { return nullptr; } for (size_t i = 0; i < numFuncs; i++) { - if (!metadata_->debugFuncArgTypes[i].appendAll( - moduleEnv_->funcs[i].type->args())) { - return nullptr; - } - if (!metadata_->debugFuncReturnTypes[i].appendAll( - moduleEnv_->funcs[i].type->results())) { - return nullptr; - } + metadata_->debugFuncTypeIndices[i] = moduleEnv_->funcs[i].typeIndex; } static_assert(sizeof(ModuleHash) <= sizeof(mozilla::SHA1Sum::Hash), diff --git a/js/src/wasm/WasmInstance.cpp b/js/src/wasm/WasmInstance.cpp index b383a03a8708..900abd09bf62 100644 --- a/js/src/wasm/WasmInstance.cpp +++ b/js/src/wasm/WasmInstance.cpp @@ -1404,7 +1404,7 @@ bool Instance::init(JSContext* cx, const JSFunctionVector& funcImports, // Add debug filtering table. if (metadata().debugEnabled) { - size_t numFuncs = metadata().debugFuncReturnTypes.length(); + size_t numFuncs = metadata().debugNumFuncs(); size_t numWords = std::max((numFuncs + 31) / 32, 1); debugFilter_ = (uint32_t*)js_calloc(numWords, sizeof(uint32_t)); if (!debugFilter_) { diff --git a/js/src/wasm/WasmSerialize.cpp b/js/src/wasm/WasmSerialize.cpp index a76201f80965..9cd75d7c216d 100644 --- a/js/src/wasm/WasmSerialize.cpp +++ b/js/src/wasm/WasmSerialize.cpp @@ -794,8 +794,7 @@ CoderResult CodeMetadata(Coder& coder, CoderArg item) { WASM_VERIFY_SERIALIZATION_FOR_SIZE(wasm::Metadata, 464); if constexpr (mode == MODE_ENCODE) { - MOZ_ASSERT(!item->debugEnabled && item->debugFuncArgTypes.empty() && - item->debugFuncReturnTypes.empty()); + MOZ_ASSERT(!item->debugEnabled && item->debugFuncTypeIndices.empty()); MOZ_ASSERT(!item->isAsmJS()); } @@ -814,8 +813,7 @@ CoderResult CodeMetadata(Coder& coder, if constexpr (mode == MODE_DECODE) { item->debugEnabled = false; - item->debugFuncArgTypes.clear(); - item->debugFuncReturnTypes.clear(); + item->debugFuncTypeIndices.clear(); } return Ok(); diff --git a/js/src/wasm/WasmStubs.cpp b/js/src/wasm/WasmStubs.cpp index 7c96e1ddd699..f7811c51e88e 100644 --- a/js/src/wasm/WasmStubs.cpp +++ b/js/src/wasm/WasmStubs.cpp @@ -32,6 +32,7 @@ #include "wasm/WasmInstance.h" #include "jit/MacroAssembler-inl.h" +#include "wasm/WasmInstance-inl.h" using namespace js; using namespace js::jit;