From e2f61fef35ba429c679f47f60df5e3472da9e8ce Mon Sep 17 00:00:00 2001 From: Matthew Gaudet Date: Wed, 4 Mar 2020 23:34:41 +0000 Subject: [PATCH] Bug 1592103 - Defer allocation of Environment Shapes to Scope allocation time r=tcampbell This changes the order of the arguments to ScopeCreationData's constructor to handle the fact that EnvironmentShapeCreationData, because it contains a Variant, is MOZ_NON_PARAM, and therefore cannot be passed as an argument, or contain a default argument. Differential Revision: https://phabricator.services.mozilla.com/D52778 --HG-- extra : moz-landing-system : lando --- js/public/GCPolicyAPI.h | 3 + js/src/frontend/Stencil.cpp | 35 +++++++++- js/src/frontend/Stencil.h | 73 ++++++++++++++++--- js/src/vm/Scope.cpp | 135 +++++++++++++++++++++++++++++------- js/src/vm/Scope.h | 49 ++++++++++--- 5 files changed, 246 insertions(+), 49 deletions(-) diff --git a/js/public/GCPolicyAPI.h b/js/public/GCPolicyAPI.h index 6b369f923a74..894ba13a14fd 100644 --- a/js/public/GCPolicyAPI.h +++ b/js/public/GCPolicyAPI.h @@ -200,6 +200,9 @@ struct GCPolicy> { } }; +template <> +struct GCPolicy : public IgnoreGCPolicy {}; + // GCPolicy> forwards tracing/sweeping to GCPolicy if // when the Maybe is full. template diff --git a/js/src/frontend/Stencil.cpp b/js/src/frontend/Stencil.cpp index 276e5dd761d5..2c5ee74b0f03 100644 --- a/js/src/frontend/Stencil.cpp +++ b/js/src/frontend/Stencil.cpp @@ -14,6 +14,35 @@ using namespace js; using namespace js::frontend; +bool frontend::EnvironmentShapeCreationData::createShape( + JSContext* cx, MutableHandleShape shape) { + struct Matcher { + JSContext* cx; + MutableHandleShape& shape; + + bool operator()(CreateEnvShapeData& data) { + shape.set(CreateEnvironmentShape(cx, data.freshBi, data.cls, + data.nextEnvironmentSlot, + data.baseShapeFlags)); + return shape; + } + + bool operator()(EmptyEnvShapeData& data) { + shape.set(EmptyEnvironmentShape(cx, data.cls, JSSLOT_FREE(data.cls), + data.baseShapeFlags)); + return shape; + } + + bool operator()(mozilla::Nothing&) { + shape.set(nullptr); + return true; + } + }; + + Matcher m{cx, shape}; + return data_.match(m); +} + Scope* ScopeCreationData::createScope(JSContext* cx) { // If we've already created a scope, best just return it. if (scope_) { @@ -68,9 +97,9 @@ void ScopeCreationData::trace(JSTracer* trc) { if (enclosing_) { enclosing_.trace(trc); } - if (environmentShape_) { - TraceEdge(trc, &environmentShape_, "ScopeCreationData Environment Shape"); - } + + environmentShape_.trace(trc); + if (scope_) { TraceEdge(trc, &scope_, "ScopeCreationData Scope"); } diff --git a/js/src/frontend/Stencil.h b/js/src/frontend/Stencil.h index a66151ac48aa..7b1e782bbff5 100644 --- a/js/src/frontend/Stencil.h +++ b/js/src/frontend/Stencil.h @@ -21,6 +21,7 @@ #include "gc/AllocKind.h" // gc::AllocKind #include "gc/Barrier.h" // HeapPtr, GCPtrAtom #include "gc/Rooting.h" // HandleAtom, HandleModuleObject, HandleScriptSourceObject, MutableHandleScope +#include "js/GCVariant.h" // GC Support for mozilla::Variant #include "js/RegExpFlags.h" // JS::RegExpFlags #include "js/RootingAPI.h" // Handle #include "js/UniquePtr.h" // UniquePtr @@ -228,6 +229,57 @@ class BigIntCreationData { using BigIntIndex = TypedIndex; +class EnvironmentShapeCreationData { + // Data used to call CreateEnvShapeData + struct CreateEnvShapeData { + BindingIter freshBi; + const JSClass* cls; + uint32_t nextEnvironmentSlot; + uint32_t baseShapeFlags; + + void trace(JSTracer* trc) { freshBi.trace(trc); } + }; + + // Data used to call EmptyEnvironmentShape + struct EmptyEnvShapeData { + const JSClass* cls; + uint32_t baseShapeFlags; + void trace(JSTracer* trc){ + // Rather than having to expose this type as public to provide + // an IgnoreGCPolicy, just have an empty trace. + }; + }; + + // Three different paths to creating an environment shape: Either we produce + // nullptr directly (represented by storing Nothing in the variant), or we + // call CreateEnvironmentShape, or we call EmptyEnvironmentShape. + mozilla::Variant + data_ = mozilla::AsVariant(mozilla::Nothing()); + + public: + explicit operator bool() const { return !data_.is(); } + + // Setup for calling CreateEnvironmentShape + void set(const BindingIter& freshBi, const JSClass* cls, + uint32_t nextEnvironmentSlot, uint32_t baseShapeFlags) { + data_ = mozilla::AsVariant( + CreateEnvShapeData{freshBi, cls, nextEnvironmentSlot, baseShapeFlags}); + } + + // Setup for calling EmptyEnviornmentShape + void set(const JSClass* cls, uint32_t shapeFlags) { + data_ = mozilla::AsVariant(EmptyEnvShapeData{cls, shapeFlags}); + } + + // Reifiy this into an actual shape. + MOZ_MUST_USE bool createShape(JSContext* cx, MutableHandleShape shape); + + void trace(JSTracer* trc) { + using DataGCPolicy = JS::GCPolicy; + DataGCPolicy::trace(trc, &data_, "data_"); + } +}; + class ScopeCreationData { friend class js::AbstractScope; friend class js::GCMarker; @@ -238,9 +290,8 @@ class ScopeCreationData { // The kind determines data_. ScopeKind kind_; - // If there are any aliased bindings, the shape for the - // EnvironmentObject. Otherwise nullptr. - HeapPtr environmentShape_ = {}; + // Data to reify an environment shape at creation time. + EnvironmentShapeCreationData environmentShape_; // Once we've produced a scope from a scope creation data, there may still be // AbstractScopes refering to this ScopeCreationData, and if reification is @@ -258,14 +309,14 @@ class ScopeCreationData { UniquePtr data_; public: - ScopeCreationData(JSContext* cx, ScopeKind kind, - Handle enclosing, - UniquePtr data = {}, - Shape* environmentShape = nullptr, - frontend::FunctionBox* funbox = nullptr) + ScopeCreationData( + JSContext* cx, ScopeKind kind, Handle enclosing, + Handle environmentShape, + UniquePtr data = {}, + frontend::FunctionBox* funbox = nullptr) : enclosing_(enclosing), kind_(kind), - environmentShape_(environmentShape), + environmentShape_(environmentShape), // Copied funbox_(funbox), data_(std::move(data)) {} @@ -314,7 +365,9 @@ class ScopeCreationData { Handle enclosing, ScopeIndex* index); bool hasEnvironment() const { - return Scope::hasEnvironment(kind(), environmentShape_); + // Check if scope kind alone means we have an env shape, and + // otherwise check if we have one created. + return Scope::hasEnvironment(kind(), !!environmentShape_); } // Valid for functions; diff --git a/js/src/vm/Scope.cpp b/js/src/vm/Scope.cpp index 97e410c5a4fa..0f5bc4d42832 100644 --- a/js/src/vm/Scope.cpp +++ b/js/src/vm/Scope.cpp @@ -117,9 +117,9 @@ static Shape* NextEnvironmentShape(JSContext* cx, HandleAtom name, return cx->zone()->propertyTree().getChild(cx, shape, child); } -static Shape* CreateEnvironmentShape(JSContext* cx, BindingIter& bi, - const JSClass* cls, uint32_t numSlots, - uint32_t baseShapeFlags) { +Shape* js::CreateEnvironmentShape(JSContext* cx, BindingIter& bi, + const JSClass* cls, uint32_t numSlots, + uint32_t baseShapeFlags) { RootedShape shape(cx, EmptyEnvironmentShape(cx, cls, numSlots, baseShapeFlags)); if (!shape) { @@ -184,11 +184,19 @@ static bool SetEnvironmentShape(JSContext* cx, BindingIter& freshBi, return envShape; } -template +static bool SetEnvironmentShape( + JSContext* cx, BindingIter& freshBi, BindingIter& bi, const JSClass* cls, + uint32_t baseShapeFlags, + MutableHandle envShape) { + envShape.get().set(freshBi, cls, bi.nextEnvironmentSlot(), baseShapeFlags); + return true; +} + +template static bool PrepareScopeData( JSContext* cx, BindingIter& bi, Handle> data, const JSClass* cls, - uint32_t baseShapeFlags, MutableHandleShape envShape) { + uint32_t baseShapeFlags, ShapeType envShape) { // Copy a fresh BindingIter for use below. BindingIter freshBi(bi); @@ -632,11 +640,12 @@ uint32_t LexicalScope::nextFrameSlot(const AbstractScope& scope) { MOZ_CRASH("Not an enclosing intra-frame Scope"); } +template bool LexicalScope::prepareForScopeCreation(JSContext* cx, ScopeKind kind, uint32_t firstFrameSlot, Handle enclosing, MutableHandle> data, - MutableHandleShape envShape) { + ShapeType envShape) { bool isNamedLambda = kind == ScopeKind::NamedLambda || kind == ScopeKind::StrictNamedLambda; @@ -739,10 +748,11 @@ template static constexpr uint32_t FunctionScopeEnvShapeFlags = BaseShape::QUALIFIED_VAROBJ | BaseShape::DELEGATE; +template bool FunctionScope::prepareForScopeCreation( JSContext* cx, MutableHandle> data, bool hasParameterExprs, IsFieldInitializer isFieldInitializer, bool needsEnvironment, - HandleFunction fun, MutableHandleShape envShape) { + HandleFunction fun, ShapeType envShape) { BindingIter bi(*data, hasParameterExprs); uint32_t shapeFlags = FunctionScopeEnvShapeFlags; if (!PrepareScopeData(cx, bi, data, &CallObject::class_, @@ -776,6 +786,27 @@ bool FunctionScope::updateEnvShapeIfRequired(JSContext* cx, return true; } +bool FunctionScope::updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape, + bool needsEnvironment, bool hasParameterExprs) { + // An environment may be needed regardless of existence of any closed over + // bindings: + // - Extensible scopes (i.e., due to direct eval) + // - Needing a home object + // - Being a derived class constructor + // - Being a generator + if (!envShape.get() && needsEnvironment) { + const JSClass* cls = &CallObject::class_; + uint32_t shapeFlags = FunctionScopeEnvShapeFlags; + envShape.get().set(cls, shapeFlags); + if (!envShape.get()) { + return false; + } + } + return true; +} + /* static */ FunctionScope* FunctionScope::createWithData( JSContext* cx, MutableHandle> data, bool hasParameterExprs, @@ -925,11 +956,12 @@ static UniquePtr NewEmptyVarScopeData(JSContext* cx, return data; } +template bool VarScope::prepareForScopeCreation(JSContext* cx, ScopeKind kind, MutableHandle> data, uint32_t firstFrameSlot, bool needsEnvironment, - MutableHandleShape envShape) { + ShapeType envShape) { BindingIter bi(*data, firstFrameSlot); if (!PrepareScopeData(cx, bi, data, &VarEnvironmentObject::class_, VarScopeEnvShapeFlags, envShape)) { @@ -955,6 +987,23 @@ bool VarScope::updateEnvShapeIfRequired(JSContext* cx, return true; } +bool VarScope::updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape, + bool needsEnvironment) { + // An environment may be needed regardless of existence of any closed over + // bindings: + // - Extensible scopes (i.e., due to direct eval) + // - Being a generator + if (!envShape.get() && needsEnvironment) { + envShape.get().set(&VarEnvironmentObject::class_, VarScopeEnvShapeFlags); + if (!envShape.get()) { + return false; + } + } + return true; +} + /* static */ VarScope* VarScope::createWithData(JSContext* cx, ScopeKind kind, MutableHandle> data, @@ -1166,9 +1215,10 @@ template static const uint32_t EvalScopeEnvShapeFlags = BaseShape::QUALIFIED_VAROBJ | BaseShape::DELEGATE; +template bool EvalScope::prepareForScopeCreation(JSContext* cx, ScopeKind scopeKind, MutableHandle> data, - MutableHandleShape envShape) { + ShapeType envShape) { if (scopeKind == ScopeKind::StrictEval) { BindingIter bi(*data, true); if (!PrepareScopeData(cx, bi, data, @@ -1195,6 +1245,21 @@ bool EvalScope::updateEnvShapeIfRequired(JSContext* cx, return true; } +bool EvalScope::updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape, + ScopeKind scopeKind) { + // Strict eval and direct eval in parameter expressions always get their own + // var environment even if there are no bindings. + if (!envShape.get() && scopeKind == ScopeKind::StrictEval) { + envShape.get().set(&VarEnvironmentObject::class_, EvalScopeEnvShapeFlags); + if (!envShape.get()) { + return false; + } + } + return true; +} + /* static */ EvalScope* EvalScope::createWithData(JSContext* cx, ScopeKind scopeKind, MutableHandle> data, @@ -1282,10 +1347,11 @@ Zone* ModuleScope::Data::zone() const { } /* static */ +template bool ModuleScope::prepareForScopeCreation(JSContext* cx, MutableHandle> data, HandleModuleObject module, - MutableHandleShape envShape) { + ShapeType envShape) { BindingIter bi(*data); if (!PrepareScopeData(cx, bi, data, &ModuleEnvironmentObject::class_, @@ -1313,6 +1379,20 @@ bool ModuleScope::updateEnvShapeIfRequired(JSContext* cx, return true; } +bool ModuleScope::updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape) { + // Modules always need an environment object for now. + if (!envShape.get()) { + envShape.get().set(&ModuleEnvironmentObject::class_, + ModuleScopeEnvShapeFlags); + if (!envShape.get()) { + return false; + } + } + return true; +} + /* static */ ModuleScope* ModuleScope::createWithData(JSContext* cx, MutableHandle> data, @@ -1846,8 +1926,8 @@ bool ScopeCreationData::create(JSContext* cx, return false; } - RootedShape envShape(cx); RootedFunction fun(cx, funbox->function()); + Rooted envShape(cx); if (!FunctionScope::prepareForScopeCreation( cx, &data, hasParameterExprs, dataArg ? dataArg->isFieldInitializer : IsFieldInitializer::No, @@ -1857,7 +1937,7 @@ bool ScopeCreationData::create(JSContext* cx, *index = compilationInfo.scopeCreationData.length(); return compilationInfo.scopeCreationData.emplaceBack( - cx, ScopeKind::Function, enclosing, std::move(data.get()), envShape, + cx, ScopeKind::Function, enclosing, envShape, std::move(data.get()), funbox); } @@ -1874,7 +1954,7 @@ bool ScopeCreationData::create( return false; } - RootedShape envShape(cx); + Rooted envShape(cx); if (!LexicalScope::prepareForScopeCreation(cx, kind, firstFrameSlot, enclosing, &data, &envShape)) { return false; @@ -1882,7 +1962,7 @@ bool ScopeCreationData::create( *index = compilationInfo.scopeCreationData.length(); return compilationInfo.scopeCreationData.emplaceBack( - cx, kind, enclosing, std::move(data.get()), envShape); + cx, kind, enclosing, envShape, std::move(data.get())); } bool ScopeCreationData::create(JSContext* cx, @@ -1900,7 +1980,7 @@ bool ScopeCreationData::create(JSContext* cx, return false; } - RootedShape envShape(cx); + Rooted envShape(cx); if (!VarScope::prepareForScopeCreation(cx, kind, &data, firstFrameSlot, needsEnvironment, &envShape)) { return false; @@ -1908,7 +1988,7 @@ bool ScopeCreationData::create(JSContext* cx, *index = compilationInfo.scopeCreationData.length(); return compilationInfo.scopeCreationData.emplaceBack( - cx, kind, enclosing, std::move(data.get()), envShape); + cx, kind, enclosing, envShape, std::move(data.get())); } /* static */ @@ -1930,11 +2010,12 @@ bool ScopeCreationData::create(JSContext* cx, // global lexical scope and the global object or non-syntactic objects // created by embedding, all of which are not only extensible but may // have names on them deleted. + Rooted environmentShape(cx); Rooted enclosing(cx); *index = compilationInfo.scopeCreationData.length(); - return compilationInfo.scopeCreationData.emplaceBack(cx, kind, enclosing, - std::move(data.get())); + return compilationInfo.scopeCreationData.emplaceBack( + cx, kind, enclosing, environmentShape, std::move(data.get())); } /* static */ @@ -1952,14 +2033,14 @@ bool ScopeCreationData::create(JSContext* cx, return false; } - RootedShape envShape(cx); + Rooted envShape(cx); if (!EvalScope::prepareForScopeCreation(cx, kind, &data, &envShape)) { return false; } *index = compilationInfo.scopeCreationData.length(); return compilationInfo.scopeCreationData.emplaceBack( - cx, kind, enclosing, std::move(data.get()), envShape); + cx, kind, enclosing, envShape, std::move(data.get())); } /* static */ @@ -1982,14 +2063,14 @@ bool ScopeCreationData::create(JSContext* cx, // The data that's passed in is from the frontend and is LifoAlloc'd. // Copy it now that we're creating a permanent VM scope. - RootedShape envShape(cx); + Rooted envShape(cx); if (!ModuleScope::prepareForScopeCreation(cx, &data, module, &envShape)) { return false; } *index = compilationInfo.scopeCreationData.length(); return compilationInfo.scopeCreationData.emplaceBack( - cx, ScopeKind::Module, enclosing, std::move(data.get()), envShape); + cx, ScopeKind::Module, enclosing, envShape, std::move(data.get())); } /* static */ @@ -1998,8 +2079,9 @@ bool ScopeCreationData::create(JSContext* cx, Handle enclosing, ScopeIndex* index) { *index = compilationInfo.scopeCreationData.length(); - return compilationInfo.scopeCreationData.emplaceBack(cx, ScopeKind::With, - enclosing); + Rooted environmentShape(cx); + return compilationInfo.scopeCreationData.emplaceBack( + cx, ScopeKind::With, enclosing, environmentShape); } // WithScopes are unique because they don't go through the @@ -2027,8 +2109,11 @@ template Scope* ScopeCreationData::createSpecificScope(JSContext* cx) { Rooted> rootedData( cx, releaseData()); - RootedShape shape(cx, environmentShape_); + RootedShape shape(cx); + if (!environmentShape_.createShape(cx, &shape)) { + return nullptr; + } RootedScope enclosingScope(cx); if (!getOrCreateEnclosingScope(cx, &enclosingScope)) { return nullptr; diff --git a/js/src/vm/Scope.h b/js/src/vm/Scope.h index cf9e657510ca..2c7f727a0c58 100644 --- a/js/src/vm/Scope.h +++ b/js/src/vm/Scope.h @@ -28,7 +28,8 @@ namespace js { namespace frontend { class ScopeCreationData; -}; +class EnvironmentShapeCreationData; +}; // namespace frontend class BaseScopeData; class ModuleObject; @@ -312,7 +313,7 @@ class Scope : public js::gc::TenuredCell { Shape* environmentShape() const { return environmentShape_; } - static bool hasEnvironment(ScopeKind kind, Shape* environmentShape) { + static bool hasEnvironment(ScopeKind kind, bool environmentShape) { switch (kind) { case ScopeKind::With: case ScopeKind::Global: @@ -320,7 +321,7 @@ class Scope : public js::gc::TenuredCell { return true; default: // If there's a shape, an environment must be created for this scope. - return environmentShape != nullptr; + return environmentShape; } } @@ -437,11 +438,12 @@ class LexicalScope : public Scope { uint32_t firstFrameSlot, HandleScope enclosing); + template static bool prepareForScopeCreation(JSContext* cx, ScopeKind kind, uint32_t firstFrameSlot, Handle enclosing, MutableHandle> data, - MutableHandleShape envShape); + ShapeType envShape); Data& data() { return *static_cast(data_); } @@ -553,17 +555,23 @@ class FunctionScope : public Scope { void trace(JSTracer* trc); }; + template static bool prepareForScopeCreation(JSContext* cx, MutableHandle> data, bool hasParameterExprs, IsFieldInitializer isFieldInitializer, bool needsEnvironment, HandleFunction fun, - MutableHandleShape envShape); + ShapeType envShape); static bool updateEnvShapeIfRequired(JSContext* cx, MutableHandleShape shape, bool needsEnvironment, bool hasParameterExprs); + static bool updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle shape, + bool needsEnvironment, bool hasParameterExprs); + static FunctionScope* clone(JSContext* cx, Handle scope, HandleFunction fun, HandleScope enclosing); @@ -650,15 +658,20 @@ class VarScope : public Scope { uint32_t firstFrameSlot, bool needsEnvironment, HandleScope enclosing); + template static bool prepareForScopeCreation(JSContext* cx, ScopeKind kind, MutableHandle> data, uint32_t firstFrameSlot, bool needsEnvironment, - MutableHandleShape envShape); + ShapeType envShape); static bool updateEnvShapeIfRequired(JSContext* cx, MutableHandleShape envShape, bool needsEnvironment); + static bool updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape, + bool needsEnvironment); Data& data() { return *static_cast(data_); } const Data& data() const { return *static_cast(data_); } @@ -826,14 +839,18 @@ class EvalScope : public Scope { MutableHandle> data, HandleScope enclosing); + template static bool prepareForScopeCreation(JSContext* cx, ScopeKind scopeKind, MutableHandle> data, - MutableHandleShape envShape); + ShapeType envShape); static bool updateEnvShapeIfRequired(JSContext* cx, MutableHandleShape envShape, ScopeKind scopeKind); - + static bool updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape, + ScopeKind scopeKind); Data& data() { return *static_cast(data_); } const Data& data() const { return *static_cast(data_); } @@ -921,14 +938,17 @@ class ModuleScope : public Scope { MutableHandle> data, Handle module, HandleScope enclosing); - + template static bool prepareForScopeCreation(JSContext* cx, MutableHandle> data, HandleModuleObject module, - MutableHandleShape envShape); + ShapeType envShape); static bool updateEnvShapeIfRequired(JSContext* cx, MutableHandleShape envShape); + static bool updateEnvShapeIfRequired( + JSContext* cx, + MutableHandle envShape); Data& data() { return *static_cast(data_); } @@ -1255,7 +1275,7 @@ class BindingIter { BindingIter(EvalScope::Data& data, bool strict) { init(data, strict); } - explicit BindingIter(const BindingIter& bi) = default; + MOZ_IMPLICIT BindingIter(const BindingIter& bi) = default; bool done() const { return index_ == length_; } @@ -1520,6 +1540,13 @@ class MutableWrappedPtrOperations void operator++(int) { iter().operator++(1); } }; +Shape* CreateEnvironmentShape(JSContext* cx, BindingIter& bi, + const JSClass* cls, uint32_t numSlots, + uint32_t baseShapeFlags); + +Shape* EmptyEnvironmentShape(JSContext* cx, const JSClass* cls, + uint32_t numSlots, uint32_t baseShapeFlags); + } // namespace js namespace JS {