From 45203e1a3f02dc8b18a9f51f5df7966b6ff44b08 Mon Sep 17 00:00:00 2001 From: Neil Dhar Date: Thu, 30 Jan 2025 16:04:34 -0800 Subject: [PATCH] Add a separate HiddenClass for lazy objects Summary: `CacheNewObject` and caching properties on the prototype benefit from being able to assume that the HiddenClass for a lazy object will never compare equal to that for an ordinary object. Reviewed By: avp Differential Revision: D68677444 fbshipit-source-id: 87d01e5ebb7f7e7ec68ae707b4fd914bcd311319 --- include/hermes/VM/JSObject.h | 12 +++++ .../hermes/VM/RuntimeHermesValueFields.def | 1 + lib/VM/Callable.cpp | 52 +++++++++++-------- lib/VM/JSObject.cpp | 3 ++ lib/VM/Runtime.cpp | 5 ++ 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/include/hermes/VM/JSObject.h b/include/hermes/VM/JSObject.h index d4e8ad6197a..aad356b0827 100644 --- a/include/hermes/VM/JSObject.h +++ b/include/hermes/VM/JSObject.h @@ -444,6 +444,18 @@ class JSObject : public GCCell { return clazz_; } + /// Set the hidden class of this object to \p clazz. This does not allocate + /// indirect property storage, and must only be used when the current and new + /// hidden classes do not have more properties than the direct storage. Note + /// that if used incorrectly, this can create completely invalid objects. + void setClassNoAllocPropStorageUnsafe(Runtime &runtime, HiddenClass *clazz) { + // Check that there is no prop storage already allocated, and the new class + // does not require it. + assert(!propStorage_); + assert(clazz->getNumProperties() <= DIRECT_PROPERTY_SLOTS); + clazz_.set(runtime, clazz, runtime.getHeap()); + } + /// \return the object ID. Assign one if not yet exist. This ID can be used /// in Set or Map where hashing is required. We don't assign object an ID /// until we actually need it. An exception is lazily created objects where diff --git a/include/hermes/VM/RuntimeHermesValueFields.def b/include/hermes/VM/RuntimeHermesValueFields.def index 4d8b98dd745..e1eba8e6b65 100644 --- a/include/hermes/VM/RuntimeHermesValueFields.def +++ b/include/hermes/VM/RuntimeHermesValueFields.def @@ -75,6 +75,7 @@ RUNTIME_HV_FIELD(throwTypeErrorAccessor, PropertyAccessor) RUNTIME_HV_FIELD(arrayClass, HiddenClass) RUNTIME_HV_FIELD(fastArrayClass, HiddenClass) RUNTIME_HV_FIELD(regExpMatchClass, HiddenClass) +RUNTIME_HV_FIELD(lazyObjectClass, HiddenClass) RUNTIME_HV_FIELD(iteratorPrototype, JSObject) RUNTIME_HV_FIELD(arrayIteratorPrototype, JSObject) diff --git a/lib/VM/Callable.cpp b/lib/VM/Callable.cpp index 010e68e54a9..6346be85ffc 100644 --- a/lib/VM/Callable.cpp +++ b/lib/VM/Callable.cpp @@ -116,10 +116,29 @@ std::string Callable::_snapshotNameImpl(GCCell *cell, GC &gc) { } #endif +/// \return the inferred parent of a Callable based on its \p kind. +static Handle inferredParent(Runtime &runtime, FuncKind kind) { + if (kind == FuncKind::Generator) { + return runtime.generatorFunctionPrototype; + } else if (kind == FuncKind::Async) { + return runtime.asyncFunctionPrototype; + } else { + assert(kind == FuncKind::Normal && "Unsupported function kind"); + return runtime.functionPrototype; + } +} + void Callable::defineLazyProperties(Handle fn, Runtime &runtime) { // lazy functions can be Bound or JS Functions. if (auto jsFun = Handle::dyn_vmcast(fn)) { const CodeBlock *codeBlock = jsFun->getCodeBlock(); + + // Set the actual non-lazy hidden class. + Handle newClass = runtime.getHiddenClassForPrototype( + *inferredParent(runtime, (FuncKind)codeBlock->getHeaderFlags().kind), + numOverlapSlots()); + jsFun->setClassNoAllocPropStorageUnsafe(runtime, *newClass); + // Create empty object for prototype. auto prototypeParent = Callable::isGeneratorFunction(*jsFun) ? Handle::vmcast(&runtime.generatorPrototype) @@ -147,6 +166,12 @@ void Callable::defineLazyProperties(Handle fn, Runtime &runtime) { cr != ExecutionStatus::EXCEPTION && "failed to define length and name"); (void)cr; } else if (auto nativeFun = Handle::dyn_vmcast(fn)) { + // Set the actual non-lazy hidden class. + Handle newClass = runtime.getHiddenClassForPrototype( + *inferredParent(runtime, (FuncKind)nativeFun->getFunctionInfo()->kind), + numOverlapSlots()); + nativeFun->setClassNoAllocPropStorageUnsafe(runtime, *newClass); + auto prototypeParent = Callable::isGeneratorFunction(*nativeFun) ? Handle::vmcast(&runtime.generatorPrototype) : Handle::vmcast(&runtime.objectPrototype); @@ -931,8 +956,7 @@ Handle NativeJSFunction::create( auto *cell = runtime.makeAFixed( runtime, parentHandle, - runtime.getHiddenClassForPrototype( - *parentHandle, numOverlapSlots()), + runtime.lazyObjectClass, functionPtr, funcInfo, unit); @@ -952,8 +976,7 @@ Handle NativeJSFunction::create( auto *cell = runtime.makeAFixed( runtime, parentHandle, - runtime.getHiddenClassForPrototype( - *parentHandle, numOverlapSlots()), + runtime.lazyObjectClass, parentEnvHandle, functionPtr, funcInfo, @@ -963,18 +986,6 @@ Handle NativeJSFunction::create( return selfHandle; } -/// \return the inferred parent of a Callable based on its \p kind. -static Handle inferredParent(Runtime &runtime, FuncKind kind) { - if (kind == FuncKind::Generator) { - return runtime.generatorFunctionPrototype; - } else if (kind == FuncKind::Async) { - return runtime.asyncFunctionPrototype; - } else { - assert(kind == FuncKind::Normal && "Unsupported function kind"); - return runtime.functionPrototype; - } -} - Handle NativeJSFunction::createWithInferredParent( Runtime &runtime, Handle parentEnvHandle, @@ -1066,8 +1077,7 @@ Handle NativeJSDerivedClass::create( auto *cell = runtime.makeAFixed( runtime, parentHandle, - runtime.getHiddenClassForPrototype( - *parentHandle, numOverlapSlots()), + runtime.lazyObjectClass, parentEnvHandle, functionPtr, funcInfo, @@ -1362,8 +1372,7 @@ PseudoHandle JSFunction::create( runtime, domain, parentHandle, - runtime.getHiddenClassForPrototype( - *parentHandle, numOverlapSlots()), + runtime.lazyObjectClass, envHandle, codeBlock); auto self = JSObjectInit::initToPseudoHandle(runtime, cell); @@ -1503,8 +1512,7 @@ PseudoHandle JSDerivedClass::create( runtime, domain, parentHandle, - runtime.getHiddenClassForPrototype( - *parentHandle, numOverlapSlots()), + runtime.lazyObjectClass, envHandle, codeBlock); auto self = JSObjectInit::initToPseudoHandle(runtime, cell); diff --git a/lib/VM/JSObject.cpp b/lib/VM/JSObject.cpp index c1ec6d642f3..2ebd92fa0d1 100644 --- a/lib/VM/JSObject.cpp +++ b/lib/VM/JSObject.cpp @@ -123,6 +123,9 @@ void JSObject::initializeLazyObject( Runtime &runtime, Handle lazyObject) { assert(lazyObject->flags_.lazyObject && "object must be lazy"); + assert( + lazyObject->getClass(runtime) == *runtime.lazyObjectClass && + "lazy object must have lazy class"); // object is now assumed to be a regular object. lazyObject->flags_.lazyObject = 0; diff --git a/lib/VM/Runtime.cpp b/lib/VM/Runtime.cpp index f51b1e04dce..cc05df27f07 100644 --- a/lib/VM/Runtime.cpp +++ b/lib/VM/Runtime.cpp @@ -396,6 +396,11 @@ Runtime::Runtime( clazz = *addResult->first; rootClazzes_[i] = clazz.getHermesValue(); } + + // Create a separate hidden class for lazy objects so that they never + // compare equal to ordinary objects. + lazyObjectClass = vmcast( + ignoreAllocationFailure(HiddenClass::createRoot(*this))); } global_ = JSObject::create(*this, makeNullHandle());