From eb83b53e50efda9d39614a7390ff0bc28b42a2b9 Mon Sep 17 00:00:00 2001 From: Natalia Csoregi Date: Thu, 28 Sep 2023 19:37:19 +0300 Subject: [PATCH] Backed out 2 changesets (bug 1855705) for causing assertion failures on Object.cpp. CLOSED TREE Backed out changeset 26356a7c39e4 (bug 1855705) Backed out changeset 36cb8b2bc8e2 (bug 1855705) --- js/src/builtin/Object.cpp | 83 +++++----------------- .../tests/basic/object-assign-plain-cache.js | 47 ------------ js/src/jit-test/tests/basic/object-assign-plain.js | 26 ------- js/src/vm/Realm.cpp | 1 - js/src/vm/Realm.h | 49 ------------- 5 files changed, 18 insertions(+), 188 deletions(-) delete mode 100644 js/src/jit-test/tests/basic/object-assign-plain-cache.js diff --git a/js/src/builtin/Object.cpp b/js/src/builtin/Object.cpp index 6a56112ea9dd..e320d6344c33 100644 --- a/js/src/builtin/Object.cpp +++ b/js/src/builtin/Object.cpp @@ -858,23 +858,6 @@ static bool CanAddNewPropertyExcludingProtoFast(PlainObject* obj) { } } -#ifdef DEBUG -void PlainObjectAssignCache::assertValid() const { - MOZ_ASSERT(emptyToShape_); - MOZ_ASSERT(fromShape_); - MOZ_ASSERT(newToShape_); - - MOZ_ASSERT(emptyToShape_->propMapLength() == 0); - MOZ_ASSERT(emptyToShape_->base() == newToShape_->base()); - MOZ_ASSERT(emptyToShape_->numFixedSlots() == newToShape_->numFixedSlots()); - - MOZ_ASSERT(emptyToShape_->getObjectClass() == &PlainObject::class_); - MOZ_ASSERT(fromShape_->getObjectClass() == &PlainObject::class_); - - MOZ_ASSERT(fromShape_->slotSpan() == newToShape_->slotSpan()); -} -#endif - [[nodiscard]] static bool TryAssignPlain(JSContext* cx, HandleObject to, HandleObject from, bool* optimized) { // Object.assign is used with PlainObjects most of the time. This is a fast @@ -906,25 +889,6 @@ void PlainObjectAssignCache::assertValid() const { return true; } - const bool toWasEmpty = toPlain->empty(); - if (toWasEmpty) { - const PlainObjectAssignCache& cache = cx->realm()->plainObjectAssignCache; - SharedShape* newShape = cache.lookup(toPlain->shape(), fromPlain->shape()); - if (newShape) { - *optimized = true; - uint32_t oldSpan = 0; - uint32_t newSpan = newShape->slotSpan(); - if (!toPlain->setShapeAndAddNewSlots(cx, newShape, oldSpan, newSpan)) { - return false; - } - MOZ_ASSERT(fromPlain->slotSpan() == newSpan); - for (size_t i = 0; i < newSpan; i++) { - toPlain->initSlot(i, fromPlain->getSlot(i)); - } - return true; - } - } - // Get a list of all enumerable |from| properties. Rooted props(cx, PropertyInfoWithKeyVector(cx)); @@ -934,7 +898,6 @@ void PlainObjectAssignCache::assertValid() const { #endif bool hasPropsWithNonDefaultAttrs = false; - bool hasOnlyEnumerableProps = true; for (ShapePropertyIter iter(fromPlain->shape()); !iter.done(); iter++) { // Symbol properties need to be assigned last. For now fall back to the // slow path if we see a symbol property. @@ -951,22 +914,18 @@ void PlainObjectAssignCache::assertValid() const { } if (iter->flags() != PropertyFlags::defaultDataPropFlags) { hasPropsWithNonDefaultAttrs = true; - if (!iter->enumerable()) { - hasOnlyEnumerableProps = false; - continue; - } + } + if (!iter->enumerable()) { + continue; } if (MOZ_UNLIKELY(!props.append(*iter))) { return false; } } - MOZ_ASSERT_IF(hasOnlyEnumerableProps, - fromPlain->slotSpan() == props.length()); - *optimized = true; - Rooted origToShape(cx, toPlain->shape()); + bool toWasEmpty = toPlain->empty(); // If the |to| object has no properties and the |from| object only has plain // enumerable/writable/configurable data properties, try to use its shape or @@ -1004,8 +963,6 @@ void PlainObjectAssignCache::assertValid() const { for (size_t i = 0; i < newSpan; i++) { toPlain->initSlot(i, fromPlain->getSlot(i)); } - PlainObjectAssignCache& cache = cx->realm()->plainObjectAssignCache; - cache.fill(&origToShape->asShared(), fromPlain->sharedShape(), newShape); return true; } } @@ -1024,29 +981,25 @@ void PlainObjectAssignCache::assertValid() const { nextKey = fromProp.key(); propValue = fromPlain->getSlot(fromProp.slot()); - if (!toWasEmpty) { - if (Maybe toProp = toPlain->lookup(cx, nextKey)) { - MOZ_ASSERT(toProp->isDataProperty()); - MOZ_ASSERT(toProp->writable()); - toPlain->setSlot(toProp->slot(), propValue); - continue; - } + Maybe toProp; + if (toWasEmpty) { + MOZ_ASSERT(!toPlain->containsPure(nextKey)); + MOZ_ASSERT(toProp.isNothing()); + } else { + toProp = toPlain->lookup(cx, nextKey); } - MOZ_ASSERT(!toPlain->containsPure(nextKey)); - - if (!AddDataPropertyToPlainObject(cx, toPlain, nextKey, propValue)) { - return false; + if (toProp.isSome()) { + MOZ_ASSERT(toProp->isDataProperty()); + MOZ_ASSERT(toProp->writable()); + toPlain->setSlot(toProp->slot(), propValue); + } else { + if (!AddDataPropertyToPlainObject(cx, toPlain, nextKey, propValue)) { + return false; + } } } - if (toWasEmpty && hasOnlyEnumerableProps && !fromPlain->inDictionaryMode() && - !toPlain->inDictionaryMode()) { - PlainObjectAssignCache& cache = cx->realm()->plainObjectAssignCache; - cache.fill(&origToShape->asShared(), fromPlain->sharedShape(), - toPlain->sharedShape()); - } - return true; } diff --git a/js/src/jit-test/tests/basic/object-assign-plain-cache.js b/js/src/jit-test/tests/basic/object-assign-plain-cache.js deleted file mode 100644 index 9936fffbdc82..000000000000 --- a/js/src/jit-test/tests/basic/object-assign-plain-cache.js +++ /dev/null @@ -1,47 +0,0 @@ -// The cache lookup must happen after ensuring there are no dense elements. -function testNewDenseElement() { - var from = {x: 1, y: 2, z: 3}; - - for (var i = 0; i < 10; i++) { - if (i === 6) { - from[0] = 1; - } - var to = Object.assign({}, from); - if (i >= 6) { - assertEq(JSON.stringify(to), '{"0":1,"x":1,"y":2,"z":3}'); - } else { - assertEq(JSON.stringify(to), '{"x":1,"y":2,"z":3}'); - } - } -} -testNewDenseElement(); - -// The cache lookup must happen after ensuring there are non-writable -// properties on the proto chain. -function testProtoNonWritable() { - var proto = {x: 1}; - var from = {x: 1, y: 2, z: 3}; - - for (var i = 0; i < 10; i++) { - if (i === 6) { - Object.freeze(proto); - } - - var to = Object.create(proto); - var ex = null; - try { - Object.assign(to, from); - } catch (e) { - ex = e; - } - - assertEq(ex instanceof TypeError, i > 5); - - if (i <= 5) { - assertEq(JSON.stringify(to), '{"x":1,"y":2,"z":3}'); - } else { - assertEq(JSON.stringify(to), '{}'); - } - } -} -testProtoNonWritable(); diff --git a/js/src/jit-test/tests/basic/object-assign-plain.js b/js/src/jit-test/tests/basic/object-assign-plain.js index cf0db7134e42..0476eb8c09f4 100644 --- a/js/src/jit-test/tests/basic/object-assign-plain.js +++ b/js/src/jit-test/tests/basic/object-assign-plain.js @@ -12,8 +12,6 @@ function testProtoSetter() { assertEq(Object.getOwnPropertyNames(to).length, 0); } testProtoSetter(); -testProtoSetter(); -testProtoSetter(); function testProtoDataProp() { var to = Object.create(null); @@ -25,8 +23,6 @@ function testProtoDataProp() { assertEq(to.__proto__, 2); } testProtoDataProp(); -testProtoDataProp(); -testProtoDataProp(); function testNonExtensible() { var to = Object.preventExtensions({x: 1}); @@ -37,16 +33,12 @@ function testNonExtensible() { assertEq("y" in to, false); } testNonExtensible(); -testNonExtensible(); -testNonExtensible(); function testNonExtensibleNoProps() { var to = Object.preventExtensions({}); Object.assign(to, {}); // No exception. } testNonExtensibleNoProps(); -testNonExtensibleNoProps(); -testNonExtensibleNoProps(); function testDenseElements() { var to = Object.assign({}, {0: 1, 1: 2}); @@ -54,8 +46,6 @@ function testDenseElements() { assertEq(to[1], 2); } testDenseElements(); -testDenseElements(); -testDenseElements(); function testNonWritableOnProto() { var proto = {}; @@ -66,8 +56,6 @@ function testNonWritableOnProto() { assertEq(Object.getOwnPropertyNames(to).length, 0); } testNonWritableOnProto(); -testNonWritableOnProto(); -testNonWritableOnProto(); function testAccessorOnProto() { var setterVal; @@ -77,8 +65,6 @@ function testAccessorOnProto() { assertEq(Object.getOwnPropertyNames(to).length, 0); } testAccessorOnProto(); -testAccessorOnProto(); -testAccessorOnProto(); function testSetAndAdd() { var to = Object.assign({x: 1, y: 2}, {x: 3, y: 4, z: 5}); @@ -87,8 +73,6 @@ function testSetAndAdd() { assertEq(to.z, 5); } testSetAndAdd(); -testSetAndAdd(); -testSetAndAdd(); function testNonConfigurableFrom() { var from = {}; @@ -98,8 +82,6 @@ function testNonConfigurableFrom() { assertEq(Object.getOwnPropertyDescriptor(to, "x").configurable, true); } testNonConfigurableFrom(); -testNonConfigurableFrom(); -testNonConfigurableFrom(); function testNonEnumerableFrom() { var from = {}; @@ -109,8 +91,6 @@ function testNonEnumerableFrom() { assertEq(to.x, undefined); } testNonEnumerableFrom(); -testNonEnumerableFrom(); -testNonEnumerableFrom(); function testNonWritableFrom() { var from = {}; @@ -120,8 +100,6 @@ function testNonWritableFrom() { assertEq(Object.getOwnPropertyDescriptor(to, "x").writable, true); } testNonWritableFrom(); -testNonWritableFrom(); -testNonWritableFrom(); function testFrozenProto() { var proto = Object.freeze({x: 1}); @@ -132,8 +110,6 @@ function testFrozenProto() { assertEq(target.x, 1); } testFrozenProto(); -testFrozenProto(); -testFrozenProto(); function testReuseShape() { var from = {}; @@ -144,5 +120,3 @@ function testReuseShape() { assertEq(to.y, 2); } testReuseShape(); -testReuseShape(); -testReuseShape(); diff --git a/js/src/vm/Realm.cpp b/js/src/vm/Realm.cpp index 4766fd3d9b75..c32018724186 100644 --- a/js/src/vm/Realm.cpp +++ b/js/src/vm/Realm.cpp @@ -315,7 +315,6 @@ void Realm::purge() { dtoaCache.purge(); newProxyCache.purge(); newPlainObjectWithPropsCache.purge(); - plainObjectAssignCache.purge(); objects_.iteratorCache.clearAndCompact(); arraySpeciesLookup.purge(); promiseLookup.purge(); diff --git a/js/src/vm/Realm.h b/js/src/vm/Realm.h index deabe4cb97a0..65f5d7311d61 100644 --- a/js/src/vm/Realm.h +++ b/js/src/vm/Realm.h @@ -136,54 +136,6 @@ class NewPlainObjectWithPropsCache { } }; -// Cache for Object.assign's fast path for two plain objects. It's used to -// optimize: -// -// Object.assign(to, from) -// -// If the |to| object has shape |emptyToShape_| (shape with no properties) and -// the |from| object has shape |fromShape_|, we can use |newToShape_| for |to| -// and copy all (data)) properties from the |from| object. -// -// This is a one-entry cache for now. It has a hit rate of > 90% on both -// Speedometer 2 and Speedometer 3. -class MOZ_NON_TEMPORARY_CLASS PlainObjectAssignCache { - SharedShape* emptyToShape_ = nullptr; - SharedShape* fromShape_ = nullptr; - SharedShape* newToShape_ = nullptr; - -#ifdef DEBUG - void assertValid() const; -#else - void assertValid() const {} -#endif - - public: - PlainObjectAssignCache() = default; - PlainObjectAssignCache(const PlainObjectAssignCache&) = delete; - void operator=(const PlainObjectAssignCache&) = delete; - - SharedShape* lookup(Shape* emptyToShape, Shape* fromShape) const { - if (emptyToShape_ == emptyToShape && fromShape_ == fromShape) { - assertValid(); - return newToShape_; - } - return nullptr; - } - void fill(SharedShape* emptyToShape, SharedShape* fromShape, - SharedShape* newToShape) { - emptyToShape_ = emptyToShape; - fromShape_ = fromShape; - newToShape_ = newToShape; - assertValid(); - } - void purge() { - emptyToShape_ = nullptr; - fromShape_ = nullptr; - newToShape_ = nullptr; - } -}; - // [SMDOC] Object MetadataBuilder API // // We must ensure that all newly allocated JSObjects get their metadata @@ -382,7 +334,6 @@ class JS::Realm : public JS::shadow::Realm { js::DtoaCache dtoaCache; js::NewProxyCache newProxyCache; js::NewPlainObjectWithPropsCache newPlainObjectWithPropsCache; - js::PlainObjectAssignCache plainObjectAssignCache; js::ArraySpeciesLookup arraySpeciesLookup; js::PromiseLookup promiseLookup; -- 2.11.4.GIT