From ece0496985800e17a853b66405232dd20b757142 Mon Sep 17 00:00:00 2001 From: Martok Date: Sat, 21 Jan 2023 22:21:45 +0100 Subject: [PATCH] Issue #2089 - Avoid copying/recreating iterator result, AsyncGeneratorRequest and GeneratorObject expression stacks Based-on: m-c 1394682,1410283,1396499 --- js/src/builtin/Promise.cpp | 7 +++- js/src/jit/BaselineCompiler.cpp | 4 +- js/src/jscompartment.cpp | 4 ++ js/src/jscompartment.h | 5 +++ js/src/jsiter.cpp | 73 +++++++++++++++++++++++++++++---- js/src/vm/AsyncIteration.cpp | 23 ++++++++--- js/src/vm/AsyncIteration.h | 63 +++++++++++++++++++++++----- js/src/vm/GeneratorObject.cpp | 39 +++++++++++++----- js/src/vm/GeneratorObject.h | 3 ++ js/src/vm/NativeObject-inl.h | 17 ++++++++ js/src/vm/NativeObject.h | 3 ++ js/src/vm/TypeInference.h | 5 +++ 12 files changed, 208 insertions(+), 38 deletions(-) diff --git a/js/src/builtin/Promise.cpp b/js/src/builtin/Promise.cpp index cd7b2c26b9..d1a92c6246 100644 --- a/js/src/builtin/Promise.cpp +++ b/js/src/builtin/Promise.cpp @@ -2861,6 +2861,8 @@ js::AsyncGeneratorResolve(JSContext* cx, Handle asyncGenO // Step 5. RootedObject resultPromise(cx, request->promise()); + asyncGenObj->cacheRequest(request); + // Step 6. RootedObject resultObj(cx, CreateIterResultObject(cx, value, done)); if (!resultObj) @@ -2899,6 +2901,8 @@ js::AsyncGeneratorReject(JSContext* cx, Handle asyncGenOb // Step 5. RootedObject resultPromise(cx, request->promise()); + asyncGenObj->cacheRequest(request); + // Step 6. if (!RejectMaybeWrappedPromise(cx, resultPromise, exception)) return false; @@ -3036,7 +3040,8 @@ js::AsyncGeneratorEnqueue(JSContext* cx, HandleValue asyncGenVal, // Step 5 (reordered). Rooted request( - cx, AsyncGeneratorRequest::create(cx, completionKind, completionValue, resultPromise)); + cx, AsyncGeneratorObject::createRequest(cx, asyncGenObj, completionKind, completionValue, + resultPromise)); if (!request) return false; diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp index 53254718c0..61de870cdb 100644 --- a/js/src/jit/BaselineCompiler.cpp +++ b/js/src/jit/BaselineCompiler.cpp @@ -4557,20 +4557,20 @@ BaselineCompiler::emit_JSOP_RESUME() Register initLength = regs.takeAny(); masm.loadPtr(Address(scratch2, NativeObject::offsetOfElements()), scratch2); masm.load32(Address(scratch2, ObjectElements::offsetOfInitializedLength()), initLength); + masm.store32(Imm32(0), Address(scratch2, ObjectElements::offsetOfInitializedLength())); Label loop, loopDone; masm.bind(&loop); masm.branchTest32(Assembler::Zero, initLength, initLength, &loopDone); { masm.pushValue(Address(scratch2, 0)); + masm.patchableCallPreBarrier(exprStackSlot, MIRType::Value); masm.addPtr(Imm32(sizeof(Value)), scratch2); masm.sub32(Imm32(1), initLength); masm.jump(&loop); } masm.bind(&loopDone); - masm.patchableCallPreBarrier(exprStackSlot, MIRType::Value); - masm.storeValue(NullValue(), exprStackSlot); regs.add(initLength); } diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 5e33f27f7d..8cf22026f3 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -85,6 +85,7 @@ JSCompartment::JSCompartment(Zone* zone, const JS::CompartmentOptions& options = jitCompartment_(nullptr), mappedArgumentsTemplate_(nullptr), unmappedArgumentsTemplate_(nullptr), + iterResultTemplate_(nullptr), lcovOutput() { runtime_->numCompartments++; @@ -846,6 +847,9 @@ JSCompartment::sweepTemplateObjects() if (unmappedArgumentsTemplate_ && IsAboutToBeFinalized(&unmappedArgumentsTemplate_)) unmappedArgumentsTemplate_.set(nullptr); + + if (iterResultTemplate_ && IsAboutToBeFinalized(&iterResultTemplate_)) + iterResultTemplate_.set(nullptr); } /* static */ void diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index d789c547fa..7723aeffa4 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -861,6 +861,7 @@ struct JSCompartment js::ReadBarriered mappedArgumentsTemplate_; js::ReadBarriered unmappedArgumentsTemplate_; + js::ReadBarriered iterResultTemplate_; public: bool ensureJitCompartmentExists(JSContext* cx); @@ -872,6 +873,10 @@ struct JSCompartment js::ArgumentsObject* maybeArgumentsTemplateObject(bool mapped) const; + static const size_t IterResultObjectValueSlot = 0; + static const size_t IterResultObjectDoneSlot = 1; + js::NativeObject* getOrCreateIterResultTemplateObject(JSContext* cx); + public: // Aggregated output used to collect JSScript hit counts when code coverage // is enabled. diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index 858e10a326..f7fb664b0c 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -8,6 +8,7 @@ #include "jsiter.h" #include "mozilla/ArrayUtils.h" +#include "mozilla/DebugOnly.h" #include "mozilla/Maybe.h" #include "mozilla/MemoryReporting.h" #include "mozilla/PodOperations.h" @@ -45,6 +46,7 @@ using namespace js::gc; using JS::ForOfIterator; using mozilla::ArrayLength; +using mozilla::DebugOnly; using mozilla::Maybe; using mozilla::PodCopy; using mozilla::PodZero; @@ -944,25 +946,78 @@ js::CreateIterResultObject(JSContext* cx, HandleValue value, bool done) // Step 1 (implicit). // Step 2. - RootedObject resultObj(cx, NewBuiltinClassInstance(cx)); - if (!resultObj) + RootedObject templateObject(cx, cx->compartment()->getOrCreateIterResultTemplateObject(cx)); + if (!templateObject) return nullptr; + NativeObject* resultObj = NativeObject::createWithTemplate(cx, gc::DefaultHeap, templateObject); + if (!resultObj) + return nullptr; + // Step 3. - if (!DefineProperty(cx, resultObj, cx->names().value, value)) - return nullptr; + resultObj->setSlot(JSCompartment::IterResultObjectValueSlot, value); // Step 4. - if (!DefineProperty(cx, resultObj, cx->names().done, - done ? TrueHandleValue : FalseHandleValue)) - { - return nullptr; - } + resultObj->setSlot(JSCompartment::IterResultObjectDoneSlot, + done ? TrueHandleValue : FalseHandleValue); // Step 5. return resultObj; } +NativeObject* +JSCompartment::getOrCreateIterResultTemplateObject(JSContext* cx) +{ + if (iterResultTemplate_) + return iterResultTemplate_; + + // Create template plain object + RootedNativeObject templateObject(cx, NewBuiltinClassInstance(cx, TenuredObject)); + if (!templateObject) + return iterResultTemplate_; // = nullptr + + // Create a new group for the template. + Rooted proto(cx, templateObject->taggedProto()); + RootedObjectGroup group(cx, ObjectGroupCompartment::makeGroup(cx, templateObject->getClass(), + proto)); + if (!group) + return iterResultTemplate_; // = nullptr + templateObject->setGroup(group); + + // Set dummy `value` property + if (!NativeDefineDataProperty(cx, templateObject, cx->names().value, UndefinedHandleValue, + JSPROP_ENUMERATE)) + { + return iterResultTemplate_; // = nullptr + } + + // Set dummy `done` property + if (!NativeDefineDataProperty(cx, templateObject, cx->names().done, TrueHandleValue, + JSPROP_ENUMERATE)) + { + return iterResultTemplate_; // = nullptr + } + + // Update `value` property typeset, since it can be any value. + HeapTypeSet* types = group->maybeGetProperty(NameToId(cx->names().value)); + MOZ_ASSERT(types); + { + AutoEnterAnalysis enter(cx); + types->makeUnknown(cx); + } + + // Make sure that the properties are in the right slots. + DebugOnly shape = templateObject->lastProperty(); + MOZ_ASSERT(shape->previous()->slot() == JSCompartment::IterResultObjectValueSlot && + shape->previous()->propidRef() == NameToId(cx->names().value)); + MOZ_ASSERT(shape->slot() == JSCompartment::IterResultObjectDoneSlot && + shape->propidRef() == NameToId(cx->names().done)); + + iterResultTemplate_.set(templateObject); + + return iterResultTemplate_; +} + bool js::ThrowStopIteration(JSContext* cx) { diff --git a/js/src/vm/AsyncIteration.cpp b/js/src/vm/AsyncIteration.cpp index bcc1814cef..58ad09815c 100644 --- a/js/src/vm/AsyncIteration.cpp +++ b/js/src/vm/AsyncIteration.cpp @@ -311,9 +311,24 @@ AsyncGeneratorObject::create(JSContext* cx, HandleFunction asyncGen, HandleValue // Step 8. asyncGenObj->clearSingleQueueRequest(); + asyncGenObj->clearCachedRequest(); + return asyncGenObj; } +/* static */ AsyncGeneratorRequest* +AsyncGeneratorObject::createRequest(JSContext* cx, Handle asyncGenObj, + CompletionKind completionKind, HandleValue completionValue, + HandleObject promise) +{ + if (!asyncGenObj->hasCachedRequest()) + return AsyncGeneratorRequest::create(cx, completionKind, completionValue, promise); + + AsyncGeneratorRequest* request = asyncGenObj->takeCachedRequest(); + request->init(completionKind, completionValue, promise); + return request; +} + static MOZ_MUST_USE bool InternalEnqueue(JSContext* cx, HandleArrayObject queue, HandleValue val) { @@ -428,17 +443,15 @@ const Class AsyncGeneratorRequest::class_ = { // Async Iteration proposal 11.4.3.1. /* static */ AsyncGeneratorRequest* -AsyncGeneratorRequest::create(JSContext* cx, CompletionKind completionKind_, - HandleValue completionValue_, HandleObject promise_) +AsyncGeneratorRequest::create(JSContext* cx, CompletionKind completionKind, + HandleValue completionValue, HandleObject promise) { RootedObject obj(cx, NewNativeObjectWithGivenProto(cx, &class_, nullptr)); if (!obj) return nullptr; Handle request = obj.as(); - request->setCompletionKind(completionKind_); - request->setCompletionValue(completionValue_); - request->setPromise(promise_); + request->init(completionKind, completionValue, promise); return request; } diff --git a/js/src/vm/AsyncIteration.h b/js/src/vm/AsyncIteration.h index b712c233a1..66c9c091f1 100644 --- a/js/src/vm/AsyncIteration.h +++ b/js/src/vm/AsyncIteration.h @@ -50,6 +50,8 @@ AsyncGeneratorYieldReturnAwaitedRejected(JSContext* cx, Handle asyncGenObj, HandleValue reason); +class AsyncGeneratorObject; + class AsyncGeneratorRequest : public NativeObject { private: @@ -60,23 +62,26 @@ class AsyncGeneratorRequest : public NativeObject Slots, }; - void setCompletionKind(CompletionKind completionKind_) { + void init(CompletionKind completionKind, HandleValue completionValue, + HandleObject promise) { setFixedSlot(Slot_CompletionKind, - Int32Value(static_cast(completionKind_))); + Int32Value(static_cast(completionKind))); + setFixedSlot(Slot_CompletionValue, completionValue); + setFixedSlot(Slot_Promise, ObjectValue(*promise)); } - void setCompletionValue(HandleValue completionValue_) { - setFixedSlot(Slot_CompletionValue, completionValue_); - } - void setPromise(HandleObject promise_) { - setFixedSlot(Slot_Promise, ObjectValue(*promise_)); + + void clearData() { + setFixedSlot(Slot_CompletionValue, NullValue()); + setFixedSlot(Slot_Promise, NullValue()); } + friend AsyncGeneratorObject; + public: static const Class class_; - static AsyncGeneratorRequest* - create(JSContext* cx, CompletionKind completionKind, HandleValue completionValue, - HandleObject promise); + static AsyncGeneratorRequest* create(JSContext* cx, CompletionKind completionKind, + HandleValue completionValue, HandleObject promise); CompletionKind completionKind() const { return static_cast(getFixedSlot(Slot_CompletionKind).toInt32()); @@ -96,6 +101,7 @@ class AsyncGeneratorObject : public NativeObject Slot_State = 0, Slot_Generator, Slot_QueueOrRequest, + Slot_CachedRequest, Slots }; @@ -139,7 +145,7 @@ class AsyncGeneratorObject : public NativeObject setFixedSlot(Slot_QueueOrRequest, ObjectValue(*request)); } void clearSingleQueueRequest() { - setFixedSlot(Slot_QueueOrRequest, NullHandleValue); + setFixedSlot(Slot_QueueOrRequest, NullValue()); } AsyncGeneratorRequest* singleQueueRequest() const { return &getFixedSlot(Slot_QueueOrRequest).toObject().as(); @@ -218,6 +224,41 @@ class AsyncGeneratorObject : public NativeObject return isSingleQueueEmpty(); return queue()->length() == 0; } + + // This function does either of the following: + // * return a cached request object with the slots updated + // * create a new request object with the slots set + static AsyncGeneratorRequest* createRequest(JSContext* cx, + Handle asyncGenObj, + CompletionKind completionKind, + HandleValue completionValue, + HandleObject promise); + + // Stores the given request to the generator's cache after clearing its data + // slots. The cached request will be reused in the subsequent createRequest + // call. + void cacheRequest(AsyncGeneratorRequest* request) { + if (hasCachedRequest()) + return; + + request->clearData(); + setFixedSlot(Slot_CachedRequest, ObjectValue(*request)); + } + + private: + bool hasCachedRequest() const { + return getFixedSlot(Slot_CachedRequest).isObject(); + } + + AsyncGeneratorRequest* takeCachedRequest() { + auto request = &getFixedSlot(Slot_CachedRequest).toObject().as(); + clearCachedRequest(); + return request; + } + + void clearCachedRequest() { + setFixedSlot(Slot_CachedRequest, NullValue()); + } }; JSObject* diff --git a/js/src/vm/GeneratorObject.cpp b/js/src/vm/GeneratorObject.cpp index df2869c89c..aad29b910d 100644 --- a/js/src/vm/GeneratorObject.cpp +++ b/js/src/vm/GeneratorObject.cpp @@ -10,7 +10,9 @@ #include "jsatominlines.h" #include "jsscriptinlines.h" +#include "vm/ArrayObject-inl.h" #include "vm/NativeObject-inl.h" +#include "vm/UnboxedObject-inl.h" #include "vm/Stack-inl.h" using namespace js; @@ -66,7 +68,7 @@ GeneratorObject::suspend(JSContext* cx, HandleObject obj, AbstractFramePtr frame MOZ_ASSERT(*pc == JSOP_INITIALYIELD || *pc == JSOP_YIELD || *pc == JSOP_AWAIT); Rooted genObj(cx, &obj->as()); - MOZ_ASSERT(!genObj->hasExpressionStack()); + MOZ_ASSERT(!genObj->hasExpressionStack() || genObj->isExpressionStackEmpty()); MOZ_ASSERT_IF(*pc == JSOP_AWAIT, genObj->callee().isAsync()); MOZ_ASSERT_IF(*pc == JSOP_YIELD, genObj->callee().isStarGenerator() || @@ -78,16 +80,33 @@ GeneratorObject::suspend(JSContext* cx, HandleObject obj, AbstractFramePtr frame return false; } + ArrayObject* stack = nullptr; + if (nvalues) { + do { + if (genObj->hasExpressionStack()) { + MOZ_ASSERT(genObj->expressionStack().getDenseInitializedLength() == 0); + auto result = SetOrExtendAnyBoxedOrUnboxedDenseElements(cx, + &genObj->expressionStack().as(), + 0, vp, nvalues, ShouldUpdateTypes::DontUpdate); + if (result == DenseElementResult::Success) { + MOZ_ASSERT(genObj->expressionStack().getDenseInitializedLength() == nvalues); + break; + } + if (result == DenseElementResult::Failure) + return false; + } + + stack = NewDenseCopiedArray(cx, nvalues, vp); + if (!stack) + return false; + } while (false); + } + uint32_t yieldAndAwaitIndex = GET_UINT24(pc); genObj->setYieldAndAwaitIndex(yieldAndAwaitIndex); genObj->setEnvironmentChain(*frame.environmentChain()); - - if (nvalues) { - ArrayObject* stack = NewDenseCopiedArray(cx, nvalues, vp); - if (!stack) - return false; + if (stack) genObj->setExpressionStack(*stack); - } return true; } @@ -167,13 +186,13 @@ GeneratorObject::resume(JSContext* cx, InterpreterActivation& activation, if (genObj->hasArgsObj()) activation.regs().fp()->initArgsObj(genObj->argsObj()); - if (genObj->hasExpressionStack()) { - uint32_t len = genObj->expressionStack().length(); + if (genObj->hasExpressionStack() && !genObj->isExpressionStackEmpty()) { + uint32_t len = genObj->expressionStack().getDenseInitializedLength(); MOZ_ASSERT(activation.regs().spForStackDepth(len)); const Value* src = genObj->expressionStack().getDenseElements(); mozilla::PodCopy(activation.regs().sp, src, len); activation.regs().sp += len; - genObj->clearExpressionStack(); + genObj->expressionStack().setDenseInitializedLength(0); } JSScript* script = callee->nonLazyScript(); diff --git a/js/src/vm/GeneratorObject.h b/js/src/vm/GeneratorObject.h index c33eb48d43..c717d25e66 100644 --- a/js/src/vm/GeneratorObject.h +++ b/js/src/vm/GeneratorObject.h @@ -99,6 +99,9 @@ class GeneratorObject : public NativeObject bool hasExpressionStack() const { return getFixedSlot(EXPRESSION_STACK_SLOT).isObject(); } + bool isExpressionStackEmpty() const { + return expressionStack().getDenseInitializedLength() == 0; + } ArrayObject& expressionStack() const { return getFixedSlot(EXPRESSION_STACK_SLOT).toObject().as(); } diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 004b308f05..4d692f4a9f 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -242,6 +242,23 @@ NativeObject::getDenseOrTypedArrayElement(uint32_t idx) return getDenseElement(idx); } +/* static */ inline NativeObject* +NativeObject::createWithTemplate(JSContext* cx, gc::InitialHeap heap, + HandleObject templateObject) +{ + RootedObjectGroup group(cx, templateObject->group()); + RootedShape shape(cx, templateObject->as().lastProperty()); + + gc::AllocKind kind = gc::GetGCObjectKind(shape->numFixedSlots()); + MOZ_ASSERT(CanBeFinalizedInBackground(kind, shape->getObjectClass())); + kind = gc::GetBackgroundAllocKind(kind); + + JSObject* baseObj = create(cx, kind, heap, shape, group); + if (!baseObj) + return nullptr; + return &baseObj->as(); +} + /* static */ inline NativeObject* NativeObject::copy(ExclusiveContext* cx, gc::AllocKind kind, gc::InitialHeap heap, HandleNativeObject templateObject) diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index cf6d684ac0..030dcfb888 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -485,6 +485,9 @@ class NativeObject : public ShapedObject return cells && cells->hasCell(cell); } + static inline NativeObject* + createWithTemplate(JSContext* cx, js::gc::InitialHeap heap, HandleObject templateObject); + protected: #ifdef DEBUG void checkShapeConsistency(); diff --git a/js/src/vm/TypeInference.h b/js/src/vm/TypeInference.h index d24a39e531..764f99ca07 100644 --- a/js/src/vm/TypeInference.h +++ b/js/src/vm/TypeInference.h @@ -620,6 +620,11 @@ class ConstraintTypeSet : public TypeSet */ void addType(ExclusiveContext* cx, Type type); + /* Generalize to any type. */ + void makeUnknown(ExclusiveContext* cx) { + addType(cx, UnknownType()); + } + // Trigger a post barrier when writing to this set, if necessary. // addType(cx, type) takes care of this automatically. void postWriteBarrier(ExclusiveContext* cx, Type type);