From 7effa6b8fe99336ce0c3d11a26d11707747cd86c Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Thu, 11 May 2017 17:12:39 +0200 Subject: [PATCH] Bug 1348772 - Optimize Array.prototype.shift to have O(1) perf instead of O(n). r=jonco JS code often uses arrays as queues, with a loop to shift() all items, and this resulted in quadratic behavior for us. That kind of code is much faster now. --- js/src/gc/Barrier.cpp | 19 ++- js/src/gc/Marking.cpp | 47 ++++--- js/src/gc/Nursery.cpp | 4 +- js/src/gc/Nursery.h | 2 +- .../jit-test/tests/basic/shifted-elements1.js | 14 +++ .../jit-test/tests/basic/shifted-elements2.js | 22 ++++ .../jit-test/tests/basic/shifted-elements3.js | 23 ++++ .../jit-test/tests/basic/shifted-elements4.js | 11 ++ .../jit-test/tests/basic/shifted-elements5.js | 39 ++++++ .../jit-test/tests/basic/shifted-elements6.js | 17 +++ js/src/jit/CodeGenerator.cpp | 32 +++-- js/src/jit/VMFunctions.cpp | 4 +- js/src/jsarray.cpp | 22 ++-- js/src/jsgc.cpp | 6 +- js/src/jsobj.cpp | 10 +- js/src/jsobjinlines.h | 3 +- js/src/vm/NativeObject-inl.h | 49 +++++++- js/src/vm/NativeObject.cpp | 117 +++++++++++++++--- js/src/vm/NativeObject.h | 99 +++++++++++++-- 19 files changed, 455 insertions(+), 85 deletions(-) create mode 100644 js/src/jit-test/tests/basic/shifted-elements1.js create mode 100644 js/src/jit-test/tests/basic/shifted-elements2.js create mode 100644 js/src/jit-test/tests/basic/shifted-elements3.js create mode 100644 js/src/jit-test/tests/basic/shifted-elements4.js create mode 100644 js/src/jit-test/tests/basic/shifted-elements5.js create mode 100644 js/src/jit-test/tests/basic/shifted-elements6.js diff --git a/js/src/gc/Barrier.cpp b/js/src/gc/Barrier.cpp index 38a1ffa97345..e35dcbe7067d 100644 --- a/js/src/gc/Barrier.cpp +++ b/js/src/gc/Barrier.cpp @@ -47,19 +47,26 @@ IsMarkedBlack(JSObject* obj) bool HeapSlot::preconditionForSet(NativeObject* owner, Kind kind, uint32_t slot) const { - return kind == Slot - ? &owner->getSlotRef(slot) == this - : &owner->getDenseElement(slot) == (const Value*)this; + if (kind == Slot) + return &owner->getSlotRef(slot) == this; + + uint32_t numShifted = owner->getElementsHeader()->numShiftedElements(); + MOZ_ASSERT(slot >= numShifted); + return &owner->getDenseElement(slot - numShifted) == (const Value*)this; } void HeapSlot::assertPreconditionForWriteBarrierPost(NativeObject* obj, Kind kind, uint32_t slot, const Value& target) const { - if (kind == Slot) + if (kind == Slot) { MOZ_ASSERT(obj->getSlotAddressUnchecked(slot)->get() == target); - else - MOZ_ASSERT(static_cast(obj->getDenseElements() + slot)->get() == target); + } else { + uint32_t numShifted = obj->getElementsHeader()->numShiftedElements(); + MOZ_ASSERT(slot >= numShifted); + MOZ_ASSERT(static_cast(obj->getDenseElements() + (slot - numShifted))->get() == + target); + } CheckEdgeIsNotBlackToGray(obj, target); } diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index feb3310b93a7..d28ffb86d25e 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -1823,7 +1823,10 @@ GCMarker::saveValueRanges() HeapSlot* vp = obj->getDenseElementsAllowCopyOnWrite(); if (array.end == vp + obj->getDenseInitializedLength()) { MOZ_ASSERT(array.start >= vp); - index = array.start - vp; + // Add the number of shifted elements here (and subtract in + // restoreValueArray) to ensure shift() calls on the array + // are handled correctly. + index = obj->unshiftedIndex(array.start - vp); kind = HeapSlot::Element; } else { HeapSlot* vp = obj->fixedSlots(); @@ -1865,6 +1868,11 @@ GCMarker::restoreValueArray(const MarkStack::SavedValueArray& array, return false; uint32_t initlen = obj->getDenseInitializedLength(); + + // Account for shifted elements. + uint32_t numShifted = obj->getElementsHeader()->numShiftedElements(); + start = (numShifted < start) ? start - numShifted : 0; + HeapSlot* vp = obj->getDenseElementsAllowCopyOnWrite(); if (start < initlen) { *vpp = vp + start; @@ -2689,8 +2697,11 @@ js::gc::StoreBuffer::SlotsEdge::trace(TenuringTracer& mover) const if (kind() == ElementKind) { int32_t initLen = obj->getDenseInitializedLength(); - int32_t clampedStart = Min(start_, initLen); - int32_t clampedEnd = Min(start_ + count_, initLen); + int32_t numShifted = obj->getElementsHeader()->numShiftedElements(); + int32_t clampedStart = Min(Max(0, start_ - numShifted), initLen); + int32_t clampedEnd = Min(Max(0, start_ + count_ - numShifted), initLen); + MOZ_ASSERT(clampedStart >= 0); + MOZ_ASSERT(clampedStart <= clampedEnd); mover.traceSlots(static_cast(obj->getDenseElements() + clampedStart) ->unsafeUnbarrieredForTracing(), clampedEnd - clampedStart); } else { @@ -3014,30 +3025,35 @@ js::TenuringTracer::moveElementsToTenured(NativeObject* dst, NativeObject* src, if (src->hasEmptyElements() || src->denseElementsAreCopyOnWrite()) return 0; - Zone* zone = src->zone(); - ObjectElements* srcHeader = src->getElementsHeader(); - ObjectElements* dstHeader; + void* srcAllocatedHeader = src->getUnshiftedElementsHeader(); /* TODO Bug 874151: Prefer to put element data inline if we have space. */ - if (!nursery().isInside(srcHeader)) { + if (!nursery().isInside(srcAllocatedHeader)) { MOZ_ASSERT(src->elements_ == dst->elements_); - nursery().removeMallocedBuffer(srcHeader); + nursery().removeMallocedBuffer(srcAllocatedHeader); return 0; } - size_t nslots = ObjectElements::VALUES_PER_HEADER + srcHeader->capacity; + ObjectElements* srcHeader = src->getElementsHeader(); + + // Shifted elements are copied too. + uint32_t numShifted = srcHeader->numShiftedElements(); + size_t nslots = srcHeader->numAllocatedElements(); /* Unlike other objects, Arrays can have fixed elements. */ if (src->is() && nslots <= GetGCKindSlots(dstKind)) { dst->as().setFixedElements(); - dstHeader = dst->as().getElementsHeader(); - js_memcpy(dstHeader, srcHeader, nslots * sizeof(HeapSlot)); - nursery().setElementsForwardingPointer(srcHeader, dstHeader, nslots); + js_memcpy(dst->getElementsHeader(), srcAllocatedHeader, nslots * sizeof(HeapSlot)); + dst->elements_ += numShifted; + nursery().setElementsForwardingPointer(srcHeader, dst->getElementsHeader(), + srcHeader->capacity); return nslots * sizeof(HeapSlot); } MOZ_ASSERT(nslots >= 2); + Zone* zone = src->zone(); + ObjectElements* dstHeader; { AutoEnterOOMUnsafeRegion oomUnsafe; dstHeader = reinterpret_cast(zone->pod_malloc(nslots)); @@ -3047,9 +3063,10 @@ js::TenuringTracer::moveElementsToTenured(NativeObject* dst, NativeObject* src, } } - js_memcpy(dstHeader, srcHeader, nslots * sizeof(HeapSlot)); - nursery().setElementsForwardingPointer(srcHeader, dstHeader, nslots); - dst->elements_ = dstHeader->elements(); + js_memcpy(dstHeader, srcAllocatedHeader, nslots * sizeof(HeapSlot)); + dst->elements_ = dstHeader->elements() + numShifted; + nursery().setElementsForwardingPointer(srcHeader, dst->getElementsHeader(), + srcHeader->capacity); return nslots * sizeof(HeapSlot); } diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index b1c02c9751be..9d183d0da0fe 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -440,11 +440,11 @@ Nursery::setSlotsForwardingPointer(HeapSlot* oldSlots, HeapSlot* newSlots, uint3 void Nursery::setElementsForwardingPointer(ObjectElements* oldHeader, ObjectElements* newHeader, - uint32_t nelems) + uint32_t capacity) { // Only use a direct forwarding pointer if there is enough space for one. setForwardingPointer(oldHeader->elements(), newHeader->elements(), - nelems > ObjectElements::VALUES_PER_HEADER); + capacity > 0); } #ifdef DEBUG diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index 6b7026755ed3..ed611571fa97 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -459,7 +459,7 @@ class Nursery void setSlotsForwardingPointer(HeapSlot* oldSlots, HeapSlot* newSlots, uint32_t nslots); void setElementsForwardingPointer(ObjectElements* oldHeader, ObjectElements* newHeader, - uint32_t nelems); + uint32_t capacity); /* Free malloced pointers owned by freed things in the nursery. */ void freeMallocedBuffers(); diff --git a/js/src/jit-test/tests/basic/shifted-elements1.js b/js/src/jit-test/tests/basic/shifted-elements1.js new file mode 100644 index 000000000000..ee001c5e3555 --- /dev/null +++ b/js/src/jit-test/tests/basic/shifted-elements1.js @@ -0,0 +1,14 @@ +function f() { + var arr = []; + var iters = 1500; + for (var i = 0; i < iters; i++) { + arr.push(i); + if (i % 2 === 0) + assertEq(arr.shift(), i / 2); + } + assertEq(arr.length, iters / 2); + for (var i = iters / 2; i < iters; i++) + assertEq(arr.shift(), i); + assertEq(arr.length, 0); +} +f(); diff --git a/js/src/jit-test/tests/basic/shifted-elements2.js b/js/src/jit-test/tests/basic/shifted-elements2.js new file mode 100644 index 000000000000..f989db1542ff --- /dev/null +++ b/js/src/jit-test/tests/basic/shifted-elements2.js @@ -0,0 +1,22 @@ +// Always use the per-element barrier. +gczeal(12); + +function f() { + var arr = []; + for (var i = 0; i < 1000; i++) + arr.push(i); + gc(); // Ensure arr is tenured. + + // Now store a nursery object somewhere in the array, shift elements, + // trigger a GC, and check the post barrier kept the object alive. + for (var i = 0; i < 20; i++) + arr.shift(); + for (var i = 0; i < 40; i++) + arr[900] = {x: i}; + for (var i = 0; i < 10; i++) + arr.shift(); + gc(); + + assertEq(arr[890].x, 39); +} +f(); diff --git a/js/src/jit-test/tests/basic/shifted-elements3.js b/js/src/jit-test/tests/basic/shifted-elements3.js new file mode 100644 index 000000000000..6bbe296faa0d --- /dev/null +++ b/js/src/jit-test/tests/basic/shifted-elements3.js @@ -0,0 +1,23 @@ +// Always use the per-element barrier. +gczeal(12); + +function f() { + var arr = []; + for (var i = 0; i < 1000; i++) + arr.push(i); + gc(); // Ensure arr is tenured. + + for (var i = 0; i < 10; i++) + arr.shift(); + + // Add a nursery object, shift all elements, and trigger a GC to ensure + // the post barrier doesn't misbehave. + for (var j = 0; j < 40; j++) + arr[500] = {x: j}; + while (arr.length > 0) + arr.shift(); + + gc(); + return arr; +} +f(); diff --git a/js/src/jit-test/tests/basic/shifted-elements4.js b/js/src/jit-test/tests/basic/shifted-elements4.js new file mode 100644 index 000000000000..59bd83608732 --- /dev/null +++ b/js/src/jit-test/tests/basic/shifted-elements4.js @@ -0,0 +1,11 @@ +function f() { + var arr = []; + for (var i = 0; i < 2; i++) { + for (var j = 0; j < 90000; j++) + arr.push(j); + for (var j = 0; j < 90000; j++) + assertEq(arr.shift(), j); + assertEq(arr.length, 0); + } +} +f(); diff --git a/js/src/jit-test/tests/basic/shifted-elements5.js b/js/src/jit-test/tests/basic/shifted-elements5.js new file mode 100644 index 000000000000..07e0bce5b52b --- /dev/null +++ b/js/src/jit-test/tests/basic/shifted-elements5.js @@ -0,0 +1,39 @@ +function testFreeze() { + var arr = []; + for (var i = 0; i < 20; i++) + arr.push(i); + for (var i = 0; i < 10; i++) + arr.shift(); + Object.freeze(arr); + assertEq(arr.length, 10); + arr[0] = -1; + assertEq(arr[0], 10); +} +testFreeze(); +testFreeze(); + +function testCopyOnWrite() { + var arr = [1, 2, 3, 4, 5, 6, 7, 8, 9]; + for (var i = 0; i < 5; i++) + assertEq(arr.shift(), i + 1); + assertEq(arr.toString(), "6,7,8,9"); +} +testCopyOnWrite(); +testCopyOnWrite(); + +function testNonWritableLength() { + var arr = []; + for (var i = 0; i < 20; i++) + arr.push(i); + Object.defineProperty(arr, "length", {writable: false, value: arr.length}); + var ex; + try { + arr.shift(); + } catch(e) { + ex = e; + } + assertEq(ex instanceof TypeError, true); + assertEq(arr.length, 20); +} +testNonWritableLength(); +testNonWritableLength(); diff --git a/js/src/jit-test/tests/basic/shifted-elements6.js b/js/src/jit-test/tests/basic/shifted-elements6.js new file mode 100644 index 000000000000..671e493254fe --- /dev/null +++ b/js/src/jit-test/tests/basic/shifted-elements6.js @@ -0,0 +1,17 @@ +// Test incremental GC slices and shifted elements. +function f() { + var arr = []; + for (var i = 0; i < 1000; i++) + arr.push({x: i}); + var arr2 = []; + for (var i = 0; i < 1000; i++) { + gcslice(900); + var o = arr.shift(); + assertEq(o.x, i); + arr2.push(o); + } + gc(); + for (var i = 0; i < 1000; i++) + assertEq(arr2[i].x, i); +} +f(); diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 489b7c2664cd..18145e737aea 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -8954,7 +8954,26 @@ CodeGenerator::emitArrayPopShift(LInstruction* lir, const MArrayPopShift* mir, R Address elementFlags(elementsTemp, ObjectElements::offsetOfFlags()); Imm32 bit(ObjectElements::NONWRITABLE_ARRAY_LENGTH); masm.branchTest32(Assembler::NonZero, elementFlags, bit, ool->entry()); + } + if (mir->mode() == MArrayPopShift::Shift) { + // Don't save the elementsTemp register. + LiveRegisterSet temps; + temps.add(elementsTemp); + + saveVolatile(temps); + masm.setupUnalignedABICall(elementsTemp); + masm.passABIArg(obj); + masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, js::ArrayShiftMoveElements)); + restoreVolatile(temps); + + if (mir->unboxedType() == JSVAL_TYPE_MAGIC) { + // Reload elementsTemp as ArrayShiftMoveElements may have moved it. + masm.loadPtr(Address(obj, NativeObject::offsetOfElements()), elementsTemp); + } + } + + if (mir->unboxedType() == JSVAL_TYPE_MAGIC) { // Now adjust length and initializedLength. masm.store32(lengthTemp, Address(elementsTemp, ObjectElements::offsetOfLength())); masm.store32(lengthTemp, Address(elementsTemp, ObjectElements::offsetOfInitializedLength())); @@ -8965,19 +8984,6 @@ CodeGenerator::emitArrayPopShift(LInstruction* lir, const MArrayPopShift* mir, R masm.add32(Imm32(-1), Address(obj, UnboxedArrayObject::offsetOfCapacityIndexAndInitializedLength())); } - if (mir->mode() == MArrayPopShift::Shift) { - // Don't save the temp registers. - LiveRegisterSet temps; - temps.add(elementsTemp); - temps.add(lengthTemp); - - saveVolatile(temps); - masm.setupUnalignedABICall(lengthTemp); - masm.passABIArg(obj); - masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, js::ArrayShiftMoveElements)); - restoreVolatile(temps); - } - masm.bind(&done); masm.bind(ool->rejoin()); } diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index 610fea957a28..028b71428799 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/js/src/jit/VMFunctions.cpp @@ -705,7 +705,9 @@ PostWriteElementBarrier(JSRuntime* rt, JSObject* obj, int32_t index) #endif ) { - rt->gc.storeBuffer().putSlot(nobj, HeapSlot::Element, index, 1); + rt->gc.storeBuffer().putSlot(nobj, HeapSlot::Element, + nobj->unshiftedIndex(index), + 1); return; } diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index b9c54c45970e..8136b9caa167 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -791,6 +791,11 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, header->initializedLength = Min(header->initializedLength, newLen); if (attrs & JSPROP_READONLY) { + if (header->numShiftedElements() > 0) { + arr->unshiftElements(); + header = arr->getElementsHeader(); + } + header->setNonwritableArrayLength(); // When an array's length becomes non-writable, writes to indexes @@ -2229,18 +2234,16 @@ ShiftMoveBoxedOrUnboxedDenseElements(JSObject* obj) { MOZ_ASSERT(HasBoxedOrUnboxedDenseElements(obj)); - /* - * At this point the length and initialized length have already been - * decremented and the result fetched, so just shift the array elements - * themselves. - */ size_t initlen = GetBoxedOrUnboxedInitializedLength(obj); + MOZ_ASSERT(initlen > 0); + if (Type == JSVAL_TYPE_MAGIC) { - obj->as().moveDenseElementsNoPreBarrier(0, 1, initlen); + if (!obj->as().tryShiftDenseElements(1)) + obj->as().moveDenseElementsNoPreBarrier(0, 1, initlen - 1); } else { uint8_t* data = obj->as().elements(); size_t elementSize = UnboxedTypeSize(Type); - memmove(data, data + elementSize, initlen * elementSize); + memmove(data, data + elementSize, (initlen - 1) * elementSize); } return DenseElementResult::Success; @@ -2275,6 +2278,11 @@ ArrayShiftDenseKernel(JSContext* cx, HandleObject obj, MutableHandleValue rval) if (rval.isMagic(JS_ELEMENTS_HOLE)) rval.setUndefined(); + if (Type == JSVAL_TYPE_MAGIC) { + if (obj->as().tryShiftDenseElements(1)) + return DenseElementResult::Success; + } + DenseElementResult result = MoveBoxedOrUnboxedDenseElements(cx, obj, 0, 1, initlen - 1); if (result != DenseElementResult::Success) return result; diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 695ffcd34385..5eb6d7544f8e 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -1971,8 +1971,10 @@ RelocateCell(Zone* zone, TenuredCell* src, AllocKind thingKind, size_t thingSize NativeObject* dstNative = &dstObj->as(); // Fixup the pointer to inline object elements if necessary. - if (srcNative->hasFixedElements()) - dstNative->setFixedElements(); + if (srcNative->hasFixedElements()) { + uint32_t numShifted = srcNative->getElementsHeader()->numShiftedElements(); + dstNative->setFixedElements(numShifted); + } // For copy-on-write objects that own their elements, fix up the // owner pointer to point to the relocated object. diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 4222cd145ef0..6f4b30199cb0 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -542,6 +542,8 @@ js::SetIntegrityLevel(JSContext* cx, HandleObject obj, IntegrityLevel level) if (level == IntegrityLevel::Frozen && obj->is()) { if (!obj->as().maybeCopyElementsForWrite(cx)) return false; + if (nobj->getElementsHeader()->numShiftedElements() > 0) + nobj->unshiftElements(); obj->as().getElementsHeader()->setNonwritableArrayLength(); } } else { @@ -3881,8 +3883,10 @@ JSObject::addSizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf, JS::ClassIn if (is() && as().hasDynamicElements()) { js::ObjectElements* elements = as().getElementsHeader(); - if (!elements->isCopyOnWrite() || elements->ownerObject() == this) - info->objectsMallocHeapElementsNormal += mallocSizeOf(elements); + if (!elements->isCopyOnWrite() || elements->ownerObject() == this) { + void* allocatedElements = as().getUnshiftedElementsHeader(); + info->objectsMallocHeapElementsNormal += mallocSizeOf(allocatedElements); + } } // Other things may be measured in the future if DMD indicates it is worthwhile. @@ -3947,7 +3951,7 @@ JSObject::sizeOfIncludingThisInNursery() const if (native.hasDynamicElements()) { js::ObjectElements& elements = *native.getElementsHeader(); if (!elements.isCopyOnWrite() || elements.ownerObject() == this) - size += elements.capacity * sizeof(HeapSlot); + size += (elements.capacity + elements.numShiftedElements()) * sizeof(HeapSlot); } if (is()) diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 75edebda82d7..d5d01bd3f708 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -99,10 +99,11 @@ JSObject::finalize(js::FreeOp* fop) // Don't free the elements until object finalization finishes, // so that other objects can access these elements while they // are themselves finalized. + MOZ_ASSERT(elements->numShiftedElements() == 0); fop->freeLater(elements); } } else { - fop->free_(elements); + fop->free_(nobj->getUnshiftedElementsHeader()); } } diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h index 2f0e171c19d6..30a7d95b5dca 100644 --- a/js/src/vm/NativeObject-inl.h +++ b/js/src/vm/NativeObject-inl.h @@ -124,7 +124,8 @@ NativeObject::elementsRangeWriteBarrierPost(uint32_t start, uint32_t count) const Value& v = elements_[start + i]; if (v.isObject() && IsInsideNursery(&v.toObject())) { zone()->group()->storeBuffer().putSlot(this, HeapSlot::Element, - start + i, count - i); + unshiftedIndex(start + i), + count - i); return; } } @@ -141,8 +142,12 @@ NativeObject::copyDenseElements(uint32_t dstStart, const Value* src, uint32_t co checkStoredValue(src[i]); #endif if (JS::shadow::Zone::asShadowZone(zone())->needsIncrementalBarrier()) { - for (uint32_t i = 0; i < count; ++i) - elements_[dstStart + i].set(this, HeapSlot::Element, dstStart + i, src[i]); + uint32_t numShifted = getElementsHeader()->numShiftedElements(); + for (uint32_t i = 0; i < count; ++i) { + elements_[dstStart + i].set(this, HeapSlot::Element, + dstStart + i + numShifted, + src[i]); + } } else { memcpy(&elements_[dstStart], src, count * sizeof(HeapSlot)); elementsRangeWriteBarrierPost(dstStart, count); @@ -163,6 +168,36 @@ NativeObject::initDenseElements(uint32_t dstStart, const Value* src, uint32_t co elementsRangeWriteBarrierPost(dstStart, count); } +inline bool +NativeObject::tryShiftDenseElements(uint32_t count) +{ + ObjectElements* header = getElementsHeader(); + if (header->isCopyOnWrite() || + header->isFrozen() || + header->hasNonwritableArrayLength() || + header->initializedLength == count) + { + return false; + } + + MOZ_ASSERT(count > 0); + MOZ_ASSERT(count < header->initializedLength); + MOZ_ASSERT(count <= ObjectElements::MaxShiftedElements); + + if (MOZ_UNLIKELY(header->numShiftedElements() + count > ObjectElements::MaxShiftedElements)) { + unshiftElements(); + header = getElementsHeader(); + } + + prepareElementRangeForOverwrite(0, count); + header->addShiftedElements(count); + + elements_ += count; + ObjectElements* newHeader = getElementsHeader(); + memmove(newHeader, header, sizeof(ObjectElements)); + return true; +} + inline void NativeObject::moveDenseElements(uint32_t dstStart, uint32_t srcStart, uint32_t count) { @@ -184,16 +219,17 @@ NativeObject::moveDenseElements(uint32_t dstStart, uint32_t srcStart, uint32_t c * the array before and after the move. */ if (JS::shadow::Zone::asShadowZone(zone())->needsIncrementalBarrier()) { + uint32_t numShifted = getElementsHeader()->numShiftedElements(); if (dstStart < srcStart) { HeapSlot* dst = elements_ + dstStart; HeapSlot* src = elements_ + srcStart; for (uint32_t i = 0; i < count; i++, dst++, src++) - dst->set(this, HeapSlot::Element, dst - elements_, *src); + dst->set(this, HeapSlot::Element, dst - elements_ + numShifted, *src); } else { HeapSlot* dst = elements_ + dstStart + count - 1; HeapSlot* src = elements_ + srcStart + count - 1; for (uint32_t i = 0; i < count; i++, dst--, src--) - dst->set(this, HeapSlot::Element, dst - elements_, *src); + dst->set(this, HeapSlot::Element, dst - elements_ + numShifted, *src); } } else { memmove(elements_ + dstStart, elements_ + srcStart, count * sizeof(HeapSlot)); @@ -231,12 +267,13 @@ NativeObject::ensureDenseInitializedLengthNoPackedCheck(JSContext* cx, uint32_t uint32_t& initlen = getElementsHeader()->initializedLength; if (initlen < index + extra) { + uint32_t numShifted = getElementsHeader()->numShiftedElements(); size_t offset = initlen; for (HeapSlot* sp = elements_ + initlen; sp != elements_ + (index + extra); sp++, offset++) { - sp->init(this, HeapSlot::Element, offset, MagicValue(JS_ELEMENTS_HOLE)); + sp->init(this, HeapSlot::Element, offset + numShifted, MagicValue(JS_ELEMENTS_HOLE)); } initlen = index + extra; } diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index c42f01b501d2..60eb2dd4fcee 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -8,6 +8,7 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/Casting.h" +#include "mozilla/CheckedInt.h" #include "jswatchpoint.h" @@ -28,6 +29,7 @@ using namespace js; using JS::AutoCheckCannotGC; using JS::GenericNaN; using mozilla::ArrayLength; +using mozilla::CheckedInt; using mozilla::DebugOnly; using mozilla::PodCopy; using mozilla::RoundUpPow2; @@ -112,6 +114,9 @@ ObjectElements::FreezeElements(JSContext* cx, HandleNativeObject obj) if (obj->hasEmptyElements()) return true; + if (obj->getElementsHeader()->numShiftedElements() > 0) + obj->unshiftElements(); + ObjectElements* header = obj->getElementsHeader(); // Note: this method doesn't update type information to indicate that the @@ -329,7 +334,7 @@ NativeObject::setLastPropertyMakeNonNative(Shape* shape) MOZ_ASSERT(shape->numFixedSlots() == 0); if (hasDynamicElements()) - js_free(getElementsHeader()); + js_free(getUnshiftedElementsHeader()); if (hasDynamicSlots()) { js_free(slots_); slots_ = nullptr; @@ -701,6 +706,49 @@ NativeObject::maybeDensifySparseElements(JSContext* cx, HandleNativeObject obj) return DenseElementResult::Success; } +void +NativeObject::unshiftElements() +{ + ObjectElements* header = getElementsHeader(); + uint32_t numShifted = header->numShiftedElements(); + MOZ_ASSERT(numShifted > 0); + + uint32_t initLength = header->initializedLength; + + ObjectElements* newHeader = static_cast(getUnshiftedElementsHeader()); + memmove(newHeader, header, sizeof(ObjectElements)); + + newHeader->clearShiftedElements(); + newHeader->capacity += numShifted; + elements_ = newHeader->elements(); + + // To move the elements, temporarily update initializedLength to include + // both shifted and unshifted elements. + newHeader->initializedLength += numShifted; + + // Move the elements. Initialize to |undefined| to ensure pre-barriers + // don't see garbage. + for (size_t i = 0; i < numShifted; i++) + initDenseElement(i, UndefinedValue()); + moveDenseElements(0, numShifted, initLength); + + // Restore the initialized length. We use setDenseInitializedLength to + // make sure prepareElementRangeForOverwrite is called on the shifted + // elements. + setDenseInitializedLength(initLength); +} + +void +NativeObject::maybeUnshiftElements() +{ + ObjectElements* header = getElementsHeader(); + MOZ_ASSERT(header->numShiftedElements() > 0); + + // Unshift if less than a third of the allocated space is in use. + if (header->capacity < header->numAllocatedElements() / 3) + unshiftElements(); +} + // Given a requested capacity (in elements) and (potentially) the length of an // array for which elements are being allocated, compute an actual allocation // amount (in elements). (Allocation amounts include space for an @@ -799,6 +847,26 @@ NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) if (denseElementsAreCopyOnWrite()) MOZ_CRASH(); + // If there are shifted elements, consider unshifting them first. If we + // don't unshift here, the code below will include the shifted elements in + // the resize. + uint32_t numShifted = getElementsHeader()->numShiftedElements(); + if (numShifted > 0) { + maybeUnshiftElements(); + if (getDenseCapacity() >= reqCapacity) + return true; + numShifted = getElementsHeader()->numShiftedElements(); + + // Ensure |reqCapacity + numShifted| below won't overflow by forcing an + // unshift in that case. + CheckedInt checkedReqCapacity(reqCapacity); + checkedReqCapacity += numShifted; + if (MOZ_UNLIKELY(!checkedReqCapacity.isValid())) { + unshiftElements(); + numShifted = 0; + } + } + uint32_t oldCapacity = getDenseCapacity(); MOZ_ASSERT(oldCapacity < reqCapacity); @@ -809,13 +877,17 @@ NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) // Preserve the |capacity <= length| invariant for arrays with // non-writable length. See also js::ArraySetLength which initially // enforces this requirement. - newAllocated = reqCapacity + ObjectElements::VALUES_PER_HEADER; + newAllocated = reqCapacity + numShifted + ObjectElements::VALUES_PER_HEADER; } else { - if (!goodElementsAllocationAmount(cx, reqCapacity, getElementsHeader()->length, &newAllocated)) + if (!goodElementsAllocationAmount(cx, reqCapacity + numShifted, + getElementsHeader()->length, + &newAllocated)) + { return false; + } } - uint32_t newCapacity = newAllocated - ObjectElements::VALUES_PER_HEADER; + uint32_t newCapacity = newAllocated - ObjectElements::VALUES_PER_HEADER - numShifted; MOZ_ASSERT(newCapacity > oldCapacity && newCapacity >= reqCapacity); // If newCapacity exceeds MAX_DENSE_ELEMENTS_COUNT, the array should become @@ -824,11 +896,11 @@ NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) uint32_t initlen = getDenseInitializedLength(); - HeapSlot* oldHeaderSlots = reinterpret_cast(getElementsHeader()); + HeapSlot* oldHeaderSlots = reinterpret_cast(getUnshiftedElementsHeader()); HeapSlot* newHeaderSlots; if (hasDynamicElements()) { MOZ_ASSERT(oldCapacity <= MAX_DENSE_ELEMENTS_COUNT); - uint32_t oldAllocated = oldCapacity + ObjectElements::VALUES_PER_HEADER; + uint32_t oldAllocated = oldCapacity + ObjectElements::VALUES_PER_HEADER + numShifted; newHeaderSlots = ReallocateObjectBuffer(cx, this, oldHeaderSlots, oldAllocated, newAllocated); if (!newHeaderSlots) @@ -837,12 +909,13 @@ NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) newHeaderSlots = AllocateObjectBuffer(cx, this, newAllocated); if (!newHeaderSlots) return false; // Leave elements at its old size. - PodCopy(newHeaderSlots, oldHeaderSlots, ObjectElements::VALUES_PER_HEADER + initlen); + PodCopy(newHeaderSlots, oldHeaderSlots, + ObjectElements::VALUES_PER_HEADER + initlen + numShifted); } ObjectElements* newheader = reinterpret_cast(newHeaderSlots); - newheader->capacity = newCapacity; - elements_ = newheader->elements(); + elements_ = newheader->elements() + numShifted; + getElementsHeader()->capacity = newCapacity; Debug_SetSlotRangeToCrashOnTouch(elements_ + initlen, newCapacity - initlen); @@ -852,9 +925,6 @@ NativeObject::growElements(JSContext* cx, uint32_t reqCapacity) void NativeObject::shrinkElements(JSContext* cx, uint32_t reqCapacity) { - uint32_t oldCapacity = getDenseCapacity(); - MOZ_ASSERT(reqCapacity < oldCapacity); - MOZ_ASSERT(canHaveNonEmptyElements()); if (denseElementsAreCopyOnWrite()) MOZ_CRASH(); @@ -862,18 +932,29 @@ NativeObject::shrinkElements(JSContext* cx, uint32_t reqCapacity) if (!hasDynamicElements()) return; + // If we have shifted elements, consider unshifting them. + uint32_t numShifted = getElementsHeader()->numShiftedElements(); + if (numShifted > 0) { + maybeUnshiftElements(); + numShifted = getElementsHeader()->numShiftedElements(); + } + + uint32_t oldCapacity = getDenseCapacity(); + MOZ_ASSERT(reqCapacity < oldCapacity); + uint32_t newAllocated = 0; - MOZ_ALWAYS_TRUE(goodElementsAllocationAmount(cx, reqCapacity, 0, &newAllocated)); + MOZ_ALWAYS_TRUE(goodElementsAllocationAmount(cx, reqCapacity + numShifted, 0, &newAllocated)); MOZ_ASSERT(oldCapacity <= MAX_DENSE_ELEMENTS_COUNT); - uint32_t oldAllocated = oldCapacity + ObjectElements::VALUES_PER_HEADER; + + uint32_t oldAllocated = oldCapacity + ObjectElements::VALUES_PER_HEADER + numShifted; if (newAllocated == oldAllocated) return; // Leave elements at its old size. MOZ_ASSERT(newAllocated > ObjectElements::VALUES_PER_HEADER); - uint32_t newCapacity = newAllocated - ObjectElements::VALUES_PER_HEADER; + uint32_t newCapacity = newAllocated - ObjectElements::VALUES_PER_HEADER - numShifted; MOZ_ASSERT(newCapacity <= MAX_DENSE_ELEMENTS_COUNT); - HeapSlot* oldHeaderSlots = reinterpret_cast(getElementsHeader()); + HeapSlot* oldHeaderSlots = reinterpret_cast(getUnshiftedElementsHeader()); HeapSlot* newHeaderSlots = ReallocateObjectBuffer(cx, this, oldHeaderSlots, oldAllocated, newAllocated); if (!newHeaderSlots) { @@ -882,8 +963,8 @@ NativeObject::shrinkElements(JSContext* cx, uint32_t reqCapacity) } ObjectElements* newheader = reinterpret_cast(newHeaderSlots); - newheader->capacity = newCapacity; - elements_ = newheader->elements(); + elements_ = newheader->elements() + numShifted; + getElementsHeader()->capacity = newCapacity; } /* static */ bool diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h index 21fc2ee45441..f4f12fc942db 100644 --- a/js/src/vm/NativeObject.h +++ b/js/src/vm/NativeObject.h @@ -157,11 +157,28 @@ ArraySetLength(JSContext* cx, Handle obj, HandleId id, * Elements do not track property creation order, so enumerating the elements * of an object does not necessarily visit indexes in the order they were * created. + * + * Shifted elements + * ---------------- + * It's pretty common to use an array as a queue, like this: + * + * while (arr.length > 0) + * foo(arr.shift()); + * + * To ensure we don't get quadratic behavior on this, elements can be 'shifted' + * in memory. tryShiftDenseElements does this by incrementing elements_ to point + * to the next element and moving the ObjectElements header in memory (so it's + * stored where the shifted Value used to be). + * + * Shifted elements can be unshifted when we grow the array, when the array is + * frozen (for simplicity, shifted elements are not supported on objects that + * are frozen, have copy-on-write elements, or on arrays with non-writable + * length). */ class ObjectElements { public: - enum Flags: uint32_t { + enum Flags: uint16_t { // Integers written to these elements must be converted to doubles. CONVERT_DOUBLE_ELEMENTS = 0x1, @@ -187,6 +204,15 @@ class ObjectElements FROZEN = 0x10, }; + // The flags word stores both the flags and the number of shifted elements. + // Allow shifting 2047 elements before unshifting. + static const size_t NumShiftedElementsBits = 11; + static const size_t MaxShiftedElements = (1 << NumShiftedElementsBits) - 1; + static const size_t NumShiftedElementsShift = 32 - NumShiftedElementsBits; + static const size_t FlagsMask = (1 << NumShiftedElementsShift) - 1; + static_assert(MaxShiftedElements == 2047, + "MaxShiftedElements should match the comment"); + private: friend class ::JSObject; friend class ArrayObject; @@ -199,7 +225,9 @@ class ObjectElements ArraySetLength(JSContext* cx, Handle obj, HandleId id, unsigned attrs, HandleValue value, ObjectOpResult& result); - /* See Flags enum above. */ + // The NumShiftedElementsBits high bits of this are used to store the + // number of shifted elements, the other bits are available for the flags. + // See Flags enum above. uint32_t flags; /* @@ -242,6 +270,21 @@ class ObjectElements flags &= ~COPY_ON_WRITE; } + void addShiftedElements(uint32_t count) { + MOZ_ASSERT(count < capacity); + MOZ_ASSERT(count < initializedLength); + MOZ_ASSERT(!(flags & (NONWRITABLE_ARRAY_LENGTH | FROZEN | COPY_ON_WRITE))); + uint32_t numShifted = numShiftedElements() + count; + MOZ_ASSERT(numShifted <= MaxShiftedElements); + flags = (numShifted << NumShiftedElementsShift) | (flags & FlagsMask); + capacity -= count; + initializedLength -= count; + } + void clearShiftedElements() { + flags &= FlagsMask; + MOZ_ASSERT(numShiftedElements() == 0); + } + public: constexpr ObjectElements(uint32_t capacity, uint32_t length) : flags(0), initializedLength(0), capacity(capacity), length(length) @@ -261,7 +304,7 @@ class ObjectElements const HeapSlot* elements() const { return reinterpret_cast(uintptr_t(this) + sizeof(ObjectElements)); } - static ObjectElements * fromElements(HeapSlot* elems) { + static ObjectElements* fromElements(HeapSlot* elems) { return reinterpret_cast(uintptr_t(elems) - sizeof(ObjectElements)); } @@ -311,6 +354,17 @@ class ObjectElements return JSPROP_ENUMERATE; } + uint32_t numShiftedElements() const { + uint32_t numShifted = flags >> NumShiftedElementsShift; + MOZ_ASSERT_IF(numShifted > 0, + !(flags & (NONWRITABLE_ARRAY_LENGTH | FROZEN | COPY_ON_WRITE))); + return numShifted; + } + + uint32_t numAllocatedElements() const { + return VALUES_PER_HEADER + capacity + numShiftedElements(); + } + // This is enough slots to store an object of this class. See the static // assertion below. static const size_t VALUES_PER_HEADER = 2; @@ -985,10 +1039,26 @@ class NativeObject : public ShapedObject "uint32_t (and sometimes int32_t ,too)"); } - ObjectElements * getElementsHeader() const { + ObjectElements* getElementsHeader() const { return ObjectElements::fromElements(elements_); } + // Returns a pointer to the first element, including shifted elements. + inline HeapSlot* unshiftedElements() const { + return elements_ - getElementsHeader()->numShiftedElements(); + } + + // Like getElementsHeader, but returns a pointer to the unshifted header. + // This is mainly useful for free()ing dynamic elements: the pointer + // returned here is the one we got from malloc. + void* getUnshiftedElementsHeader() const { + return ObjectElements::fromElements(unshiftedElements()); + } + + uint32_t unshiftedIndex(uint32_t index) const { + return index + getElementsHeader()->numShiftedElements(); + } + /* Accessors for elements. */ bool ensureElements(JSContext* cx, uint32_t capacity) { MOZ_ASSERT(!denseElementsAreCopyOnWrite()); @@ -998,6 +1068,15 @@ class NativeObject : public ShapedObject return true; } + // Try to shift |count| dense elements, see the "Shifted elements" comment. + inline bool tryShiftDenseElements(uint32_t count); + + // Unshift all shifted elements so that numShiftedElements is 0. + void unshiftElements(); + + // If this object has many shifted elements, unshift them. + void maybeUnshiftElements(); + static bool goodElementsAllocationAmount(JSContext* cx, uint32_t reqAllocated, uint32_t length, uint32_t* goodAmount); bool growElements(JSContext* cx, uint32_t newcap); @@ -1039,7 +1118,7 @@ class NativeObject : public ShapedObject MOZ_ASSERT(index < getDenseInitializedLength()); MOZ_ASSERT(!denseElementsAreCopyOnWrite()); checkStoredValue(val); - elements_[index].set(this, HeapSlot::Element, index, val); + elements_[index].set(this, HeapSlot::Element, unshiftedIndex(index), val); } public: @@ -1061,7 +1140,7 @@ class NativeObject : public ShapedObject MOZ_ASSERT(!denseElementsAreCopyOnWrite()); MOZ_ASSERT(!denseElementsAreFrozen()); checkStoredValue(val); - elements_[index].init(this, HeapSlot::Element, index, val); + elements_[index].init(this, HeapSlot::Element, unshiftedIndex(index), val); } void setDenseElementMaybeConvertDouble(uint32_t index, const Value& val) { @@ -1156,9 +1235,9 @@ class NativeObject : public ShapedObject bool canHaveNonEmptyElements(); #endif - void setFixedElements() { + void setFixedElements(uint32_t numShifted = 0) { MOZ_ASSERT(canHaveNonEmptyElements()); - elements_ = fixedElements(); + elements_ = fixedElements() + numShifted; } inline bool hasDynamicElements() const { @@ -1169,11 +1248,11 @@ class NativeObject : public ShapedObject * immediately afterwards. Such cases cannot occur for dense arrays * (which have at least two fixed slots) and can only result in a leak. */ - return !hasEmptyElements() && elements_ != fixedElements(); + return !hasEmptyElements() && !hasFixedElements(); } inline bool hasFixedElements() const { - return elements_ == fixedElements(); + return unshiftedElements() == fixedElements(); } inline bool hasEmptyElements() const {