Skip to content

Commit

Permalink
Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standa…
Browse files Browse the repository at this point in the history 
…lone. r=rhunt.

The #9 patch removed all wasm-specific fields from wasm::Metadata, but did
not remove wasm::Metadata itself, because it is inherited from by
AsmJSMetadata, and used to provide different behaviour for wasm vs asm.js in a
few obscure cases related to the profiler.

This patch restricts wasm::Metadata to be an abstract class that provides
access to (is the pure virtual base class of) AsmJSMetadata.  wasm::Metadata is
removed from WasmCode.h and instead reappears in AsmJS.h in pure virtual form.
Any place that previously took a Metadata& now takes takes a Metadata*, and
that is non-null only in the case when we are compiling asm.js.

The effect is to restrict wasm::Metadata and js::AsmJSMetadata to providing
support for asm.js compilation only.  The next patch in the series (#11)
completes the transformation by renaming those two types appropriately.

Differential Revision: https://phabricator.services.mozilla.com/D211165
  • Loading branch information
@julian-seward1
julian-seward1 committed Jun 19, 2024
1 parent 4a436c7 commit f99db70
Show file tree
Hide file tree
Showing 17 changed files with 158 additions and 159 deletions.
4 changes: 3 additions & 1 deletion js/src/debugger/Source.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,9 @@ struct DebuggerSourceGetDisplayURLMatcher {
return ss->hasDisplayURL() ? ss->displayURL() : nullptr;
}
ReturnType match(Handle<WasmInstanceObject*> wasmInstance) {
return wasmInstance->instance().metadata().displayURL();
return wasmInstance->instance().metadata()
? wasmInstance->instance().metadata()->displayURL() // asm.js
: nullptr; // wasm
}
};

Expand Down
12 changes: 9 additions & 3 deletions js/src/vm/MemoryMetrics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ struct StatsClosure {
RuntimeStats* rtStats;
ObjectPrivateVisitor* opv;
SourceSet seenSources;
wasm::Metadata::SeenSet wasmSeenMetadata;
js::Metadata::SeenSet wasmSeenMetadata;
wasm::CodeMetadata::SeenSet wasmSeenCodeMetadata;
wasm::Code::SeenSet wasmSeenCode;
wasm::Table::SeenSet wasmSeenTables;
Expand Down Expand Up @@ -346,7 +346,10 @@ static void StatsCellCallback(JSRuntime* rt, void* data, JS::GCCellPtr cellptr,
// we must be careful not to report twice.
if (obj->is<WasmModuleObject>()) {
const wasm::Module& module = obj->as<WasmModuleObject>().module();
if (ScriptSource* ss = module.metadata().maybeScriptSource()) {
ScriptSource* ss = module.metadata()
? module.metadata()->maybeScriptSource()
: nullptr;
if (ss) {
CollectScriptSourceStats<granularity>(closure, ss);
}
module.addSizeOfMisc(
Expand All @@ -355,7 +358,10 @@ static void StatsCellCallback(JSRuntime* rt, void* data, JS::GCCellPtr cellptr,
&info.objectsNonHeapCodeWasm, &info.objectsMallocHeapMisc);
} else if (obj->is<WasmInstanceObject>()) {
wasm::Instance& instance = obj->as<WasmInstanceObject>().instance();
if (ScriptSource* ss = instance.metadata().maybeScriptSource()) {
ScriptSource* ss = instance.metadata()
? instance.metadata()->maybeScriptSource()
: nullptr;
if (ss) {
CollectScriptSourceStats<granularity>(closure, ss);
}
instance.addSizeOfMisc(
Expand Down
29 changes: 17 additions & 12 deletions js/src/wasm/AsmJS.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,9 @@ struct js::AsmJSMetadata : Metadata, AsmJSMetadataCacheablePod {
}

AsmJSMetadata() : toStringStart(0), srcStart(0), strict(false) {}
~AsmJSMetadata() override = default;
~AsmJSMetadata() = default;

const AsmJSMetadata& asAsmJS() const { return *this; }

const AsmJSExport& lookupAsmJSExport(uint32_t funcIndex) const {
// The AsmJSExportVector isn't stored in sorted order so do a linear
Expand All @@ -395,14 +397,12 @@ struct js::AsmJSMetadata : Metadata, AsmJSMetadataCacheablePod {
MOZ_CRASH("missing asm.js func export");
}

bool mutedErrors() const override { return source->mutedErrors(); }
const char16_t* displayURL() const override {
bool mutedErrors() const { return source->mutedErrors(); }
const char16_t* displayURL() const {
return source->hasDisplayURL() ? source->displayURL() : nullptr;
}
ScriptSource* maybeScriptSource() const override { return source.get(); }
bool getFuncName(NameContext ctx, uint32_t funcIndex, SharedBytes namePayload,
const Maybe<Name>& moduleName, const NameVector& funcNames,
UTF8Bytes* name) const override {
ScriptSource* maybeScriptSource() const { return source.get(); }
bool getFuncNameForAsmJS(uint32_t funcIndex, UTF8Bytes* name) const {
const char* p = asmJSFuncNames[funcIndex].get();
if (!p) {
return true;
Expand All @@ -412,6 +412,11 @@ struct js::AsmJSMetadata : Metadata, AsmJSMetadataCacheablePod {

AsmJSMetadataCacheablePod& pod() { return *this; }
const AsmJSMetadataCacheablePod& pod() const { return *this; }

// FIXME: no idea if this is correct. Probably not!
size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
return 0;
}
};

using MutableAsmJSMetadata = RefPtr<AsmJSMetadata>;
Expand Down Expand Up @@ -7049,7 +7054,7 @@ bool js::InstantiateAsmJS(JSContext* cx, unsigned argc, JS::Value* vp) {

JSFunction* callee = &args.callee().as<JSFunction>();
const Module& module = AsmJSModuleFunctionToModule(callee);
const AsmJSMetadata& metadata = module.metadata().asAsmJS();
const AsmJSMetadata& metadata = module.metadata()->asAsmJS();

Rooted<WasmInstanceObject*> instanceObj(cx);
RootedObject exportObj(cx);
Expand Down Expand Up @@ -7208,11 +7213,11 @@ bool js::IsAsmJSFunction(JSFunction* fun) {

bool js::IsAsmJSStrictModeModuleOrFunction(JSFunction* fun) {
if (IsAsmJSModule(fun)) {
return AsmJSModuleFunctionToModule(fun).metadata().asAsmJS().strict;
return AsmJSModuleFunctionToModule(fun).metadata()->asAsmJS().strict;
}

if (IsAsmJSFunction(fun)) {
return ExportedFunctionToInstance(fun).metadata().asAsmJS().strict;
return ExportedFunctionToInstance(fun).metadata()->asAsmJS().strict;
}

return false;
Expand Down Expand Up @@ -7269,7 +7274,7 @@ JSString* js::AsmJSModuleToString(JSContext* cx, HandleFunction fun,
MOZ_ASSERT(IsAsmJSModule(fun));

const AsmJSMetadata& metadata =
AsmJSModuleFunctionToModule(fun).metadata().asAsmJS();
AsmJSModuleFunctionToModule(fun).metadata()->asAsmJS();
uint32_t begin = metadata.toStringStart;
uint32_t end = metadata.srcEndAfterCurly();
ScriptSource* source = metadata.maybeScriptSource();
Expand Down Expand Up @@ -7317,7 +7322,7 @@ JSString* js::AsmJSFunctionToString(JSContext* cx, HandleFunction fun) {
MOZ_ASSERT(IsAsmJSFunction(fun));

const AsmJSMetadata& metadata =
ExportedFunctionToInstance(fun).metadata().asAsmJS();
ExportedFunctionToInstance(fun).metadata()->asAsmJS();
const AsmJSExport& f =
metadata.lookupAsmJSExport(ExportedFunctionToFuncIndex(fun));

Expand Down
26 changes: 26 additions & 0 deletions js/src/wasm/AsmJS.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "jstypes.h" // JS_PUBLIC_API
#include "js/CallArgs.h" // JSNative
#include "wasm/WasmShareable.h" // ShareableBase

struct JS_PUBLIC_API JSContext;
class JS_PUBLIC_API JSFunction;
Expand All @@ -41,6 +42,7 @@ class Handle;
namespace js {

class FrontendContext;
class ScriptSource;

namespace frontend {

Expand Down Expand Up @@ -111,6 +113,30 @@ extern JSString* AsmJSModuleToString(JSContext* cx, JS::Handle<JSFunction*> fun,

extern bool IsValidAsmJSHeapLength(size_t length);

// Minimally expose wasm::Code-lifetime state in AsmJS.cpp to ModuleGenerator
// and friends.

struct AsmJSMetadata;

struct Metadata : public wasm::ShareableBase<Metadata> {
Metadata() {};
virtual ~Metadata() = default;

virtual const AsmJSMetadata& asAsmJS() const = 0;

virtual bool mutedErrors() const = 0;
virtual const char16_t* displayURL() const = 0;
virtual ScriptSource* maybeScriptSource() const = 0;
virtual bool getFuncNameForAsmJS(uint32_t funcIndex,
wasm::UTF8Bytes* name) const = 0;

virtual size_t sizeOfExcludingThis(
mozilla::MallocSizeOf mallocSizeOf) const = 0;
};

using MutableMetadata = RefPtr<Metadata>;
using SharedMetadata = RefPtr<const Metadata>;

} // namespace js

#endif // wasm_AsmJS_h
76 changes: 44 additions & 32 deletions js/src/wasm/WasmCode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ static bool AppendToString(const char* str, UTF8Bytes* bytes) {
}

static void SendCodeRangesToProfiler(const ModuleSegment& ms,
const Metadata& metadata,
const Metadata* metadata,
const CodeMetadata& codeMeta,
const CodeRangeVector& codeRanges) {
bool enabled = false;
Expand All @@ -237,9 +237,15 @@ static void SendCodeRangesToProfiler(const ModuleSegment& ms,
uintptr_t size = codeRange.end() - codeRange.begin();

UTF8Bytes name;
if (!metadata.getFuncNameStandalone(
codeRange.funcIndex(), codeMeta.namePayload, codeMeta.moduleName,
codeMeta.funcNames, &name)) {
bool ok;
if (metadata) {
ok = metadata->getFuncNameForAsmJS(codeRange.funcIndex(), &name);
} else {
ok = GetFuncNameForWasm(NameContext::Standalone, codeRange.funcIndex(),
codeMeta.namePayload, codeMeta.moduleName,
codeMeta.funcNames, &name);
}
if (!ok) {
return;
}

Expand Down Expand Up @@ -337,7 +343,7 @@ UniqueModuleSegment ModuleSegment::create(Tier tier, const Bytes& unlinkedBytes,

bool ModuleSegment::initialize(const CodeTier& codeTier,
const LinkData& linkData,
const Metadata& metadata,
const Metadata* metadata,
const CodeMetadata& codeMeta,
const MetadataTier& metadataTier) {
if (!StaticallyLink(*this, linkData)) {
Expand Down Expand Up @@ -723,10 +729,6 @@ void LazyStubTier::addSizeOfMisc(MallocSizeOf mallocSizeOf, size_t* code,
}
}

size_t Metadata::sizeOfExcludingThis(MallocSizeOf mallocSizeOf) const {
return 0;
}

struct ProjectFuncIndex {
const FuncExportVector& funcExports;
explicit ProjectFuncIndex(const FuncExportVector& funcExports)
Expand Down Expand Up @@ -779,10 +781,11 @@ static bool AppendFunctionIndexName(uint32_t funcIndex, UTF8Bytes* bytes) {
bytes->append(afterFuncIndex, strlen(afterFuncIndex));
}

bool Metadata::getFuncName(NameContext ctx, uint32_t funcIndex,
SharedBytes namePayload,
const Maybe<Name>& moduleName,
const NameVector& funcNames, UTF8Bytes* name) const {
bool js::wasm::GetFuncNameForWasm(NameContext ctx, uint32_t funcIndex,
SharedBytes namePayload,
const Maybe<Name>& moduleName,
const NameVector& funcNames,
UTF8Bytes* name) {
if (moduleName && moduleName->length != 0) {
if (!AppendName(namePayload->bytes, *moduleName, name)) {
return false;
Expand All @@ -804,7 +807,7 @@ bool Metadata::getFuncName(NameContext ctx, uint32_t funcIndex,
}

bool CodeTier::initialize(const Code& code, const LinkData& linkData,
const Metadata& metadata,
const Metadata* metadata,
const CodeMetadata& codeMeta) {
MOZ_ASSERT(!initialized());
code_ = &code;
Expand Down Expand Up @@ -890,10 +893,10 @@ bool JumpTables::init(CompileMode mode, const ModuleSegment& ms,
return true;
}

Code::Code(UniqueCodeTier tier1, const Metadata& metadata,
Code::Code(UniqueCodeTier tier1, const Metadata* metadata,
const CodeMetadata& codeMeta, JumpTables&& maybeJumpTables)
: tier1_(std::move(tier1)),
metadata_(&metadata),
metadata_(metadata),
codeMeta_(&codeMeta),
profilingLabels_(mutexid::WasmCodeProfilingLabels,
CacheableCharsVector()),
Expand All @@ -902,7 +905,7 @@ Code::Code(UniqueCodeTier tier1, const Metadata& metadata,
bool Code::initialize(const LinkData& linkData) {
MOZ_ASSERT(!initialized());

if (!tier1_->initialize(*this, linkData, *metadata_, *codeMeta_)) {
if (!tier1_->initialize(*this, linkData, metadata_, *codeMeta_)) {
return false;
}

Expand All @@ -916,7 +919,7 @@ bool Code::setAndBorrowTier2(UniqueCodeTier tier2, const LinkData& linkData,
MOZ_RELEASE_ASSERT(tier2->tier() == Tier::Optimized &&
tier1_->tier() == Tier::Baseline);

if (!tier2->initialize(*this, linkData, *metadata_, *codeMeta_)) {
if (!tier2->initialize(*this, linkData, metadata_, *codeMeta_)) {
return false;
}

Expand Down Expand Up @@ -1168,12 +1171,15 @@ void Code::ensureProfilingLabels(bool profilingEnabled) const {
MOZ_ASSERT(bytecodeStr);

UTF8Bytes name;
if (!metadata().getFuncNameStandalone(
codeRange.funcIndex(), codeMeta().namePayload,
codeMeta().moduleName, codeMeta().funcNames, &name)) {
return;
bool ok;
if (metadata()) {
ok = metadata()->getFuncNameForAsmJS(codeRange.funcIndex(), &name);
} else {
ok = GetFuncNameForWasm(NameContext::Standalone, codeRange.funcIndex(),
codeMeta().namePayload, codeMeta().moduleName,
codeMeta().funcNames, &name);
}
if (!name.append(" (", 2)) {
if (!ok || !name.append(" (", 2)) {
return;
}

Expand Down Expand Up @@ -1229,11 +1235,12 @@ void Code::addSizeOfMiscIfNotSeen(MallocSizeOf mallocSizeOf,
bool ok = seenCode->add(p, this);
(void)ok; // oh well

*data += mallocSizeOf(this) +
metadata().sizeOfIncludingThisIfNotSeen(mallocSizeOf, seenMetadata) +
codeMeta().sizeOfIncludingThisIfNotSeen(mallocSizeOf, seenCodeMeta) +
profilingLabels_.lock()->sizeOfExcludingThis(mallocSizeOf) +
jumpTables_.sizeOfMiscExcludingThis();
*data +=
mallocSizeOf(this) +
metadata()->sizeOfIncludingThisIfNotSeen(mallocSizeOf, seenMetadata) +
codeMeta().sizeOfIncludingThisIfNotSeen(mallocSizeOf, seenCodeMeta) +
profilingLabels_.lock()->sizeOfExcludingThis(mallocSizeOf) +
jumpTables_.sizeOfMiscExcludingThis();

for (auto t : tiers()) {
codeTier(t).addSizeOfMisc(mallocSizeOf, code, data);
Expand Down Expand Up @@ -1282,10 +1289,15 @@ void Code::disassemble(JSContext* cx, Tier tier, int kindSelection,
if (range.hasFuncIndex()) {
const char* funcName = "(unknown)";
UTF8Bytes namebuf;
if (metadata().getFuncNameStandalone(
range.funcIndex(), codeMeta().namePayload,
codeMeta().moduleName, codeMeta().funcNames, &namebuf) &&
namebuf.append('\0')) {
bool ok;
if (metadata()) {
ok = metadata()->getFuncNameForAsmJS(range.funcIndex(), &namebuf);
} else {
ok = GetFuncNameForWasm(NameContext::Standalone, range.funcIndex(),
codeMeta().namePayload, codeMeta().moduleName,
codeMeta().funcNames, &namebuf);
}
if (ok && namebuf.append('\0')) {
funcName = namebuf.begin();
}
SprintfLiteral(buf, "%sKind = %s, index = %d, name = %s:\n", separator,
Expand Down
Loading

0 comments on commit f99db70

Please sign in to comment.