From 533b57acdae38ce7ff0f103735f46d5216dc5a58 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Wed, 15 Apr 2020 17:23:23 +0000 Subject: [PATCH] Bug 1591276 - Track memory used by malloced buffers associated with nursery cells r=sfink This tracks the total memory used by the nursery's malloced buffers set and trigers a minor GC if it passes some limit. The limit is somewhat arbirarily defined as eight times the nursery capactity - this only fires rarely in normal use but is enough to prevent runaway memory growth in this case. Depends on D71041 Differential Revision: https://phabricator.services.mozilla.com/D71042 --HG-- extra : moz-landing-system : lando --- js/public/GCAPI.h | 4 +--- js/src/builtin/MapObject.cpp | 4 ++-- js/src/gc/Marking.cpp | 8 ++++---- js/src/gc/Nursery.cpp | 33 +++++++++++++++++++++++++-------- js/src/gc/Nursery.h | 19 ++++++++++++++++--- js/src/vm/ArgumentsObject.cpp | 4 ++-- js/src/vm/BigIntType.cpp | 11 ++++++----- js/src/vm/NativeObject.cpp | 12 ++++++------ js/src/vm/StringType-inl.h | 3 ++- js/src/vm/StringType.cpp | 8 +++++--- js/src/vm/TypedArrayObject.cpp | 2 +- 11 files changed, 70 insertions(+), 38 deletions(-) diff --git a/js/public/GCAPI.h b/js/public/GCAPI.h index 3466173436af..421c090522f4 100644 --- a/js/public/GCAPI.h +++ b/js/public/GCAPI.h @@ -471,9 +471,7 @@ namespace JS { D(TOO_MUCH_JIT_CODE, 29) \ D(FULL_CELL_PTR_BIGINT_BUFFER, 30) \ D(INIT_SELF_HOSTING, 31) \ - \ - /* These are reserved for future use. */ \ - D(RESERVED8, 32) \ + D(NURSERY_MALLOC_BUFFERS, 32) \ \ /* Reasons from Firefox */ \ D(DOM_WINDOW_UTILS, FIRST_FIREFOX_REASON) \ diff --git a/js/src/builtin/MapObject.cpp b/js/src/builtin/MapObject.cpp index 8f6e778bbe9d..c8f4cb91ea46 100644 --- a/js/src/builtin/MapObject.cpp +++ b/js/src/builtin/MapObject.cpp @@ -281,7 +281,7 @@ size_t MapIteratorObject::objectMoved(JSObject* obj, JSObject* old) { Nursery& nursery = iter->runtimeFromMainThread()->gc.nursery(); if (!nursery.isInside(range)) { - nursery.removeMallocedBuffer(range); + nursery.removeMallocedBufferDuringMinorGC(range); return 0; } @@ -1055,7 +1055,7 @@ size_t SetIteratorObject::objectMoved(JSObject* obj, JSObject* old) { Nursery& nursery = iter->runtimeFromMainThread()->gc.nursery(); if (!nursery.isInside(range)) { - nursery.removeMallocedBuffer(range); + nursery.removeMallocedBufferDuringMinorGC(range); return 0; } diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 7d47cf8de201..0148000b75ae 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -3391,7 +3391,7 @@ size_t js::TenuringTracer::moveSlotsToTenured(NativeObject* dst, if (!nursery().isInside(src->slots_)) { AddCellMemory(dst, count * sizeof(HeapSlot), MemoryUse::ObjectSlots); - nursery().removeMallocedBuffer(src->slots_); + nursery().removeMallocedBufferDuringMinorGC(src->slots_); return 0; } @@ -3428,7 +3428,7 @@ size_t js::TenuringTracer::moveElementsToTenured(NativeObject* dst, /* TODO Bug 874151: Prefer to put element data inline if we have space. */ if (!nursery().isInside(srcAllocatedHeader)) { MOZ_ASSERT(src->elements_ == dst->elements_); - nursery().removeMallocedBuffer(srcAllocatedHeader); + nursery().removeMallocedBufferDuringMinorGC(srcAllocatedHeader); AddCellMemory(dst, nslots * sizeof(HeapSlot), MemoryUse::ObjectElements); @@ -3561,7 +3561,7 @@ size_t js::TenuringTracer::moveStringToTenured(JSString* dst, JSString* src, if (src->ownsMallocedChars()) { void* chars = src->asLinear().nonInlineCharsRaw(); - nursery().removeMallocedBuffer(chars); + nursery().removeMallocedBufferDuringMinorGC(chars); AddCellMemory(dst, dst->asLinear().allocSize(), MemoryUse::StringContents); } @@ -3585,7 +3585,7 @@ size_t js::TenuringTracer::moveBigIntToTenured(JS::BigInt* dst, JS::BigInt* src, if (src->hasHeapDigits()) { size_t length = dst->digitLength(); if (!nursery().isInside(src->heapDigits_)) { - nursery().removeMallocedBuffer(src->heapDigits_); + nursery().removeMallocedBufferDuringMinorGC(src->heapDigits_); } else { Zone* zone = src->zone(); { diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 86119f3c1839..81d4c490c039 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -565,7 +565,7 @@ void* js::Nursery::allocateBuffer(Zone* zone, size_t nbytes) { } void* buffer = zone->pod_malloc(nbytes); - if (buffer && !registerMallocedBuffer(buffer)) { + if (buffer && !registerMallocedBuffer(buffer, nbytes)) { js_free(buffer); return nullptr; } @@ -607,7 +607,7 @@ void* js::Nursery::allocateZeroedBuffer( } void* buffer = zone->pod_arena_calloc(arena, nbytes); - if (buffer && !registerMallocedBuffer(buffer)) { + if (buffer && !registerMallocedBuffer(buffer, nbytes)) { js_free(buffer); return nullptr; } @@ -632,10 +632,16 @@ void* js::Nursery::reallocateBuffer(Zone* zone, Cell* cell, void* oldBuffer, } if (!isInside(oldBuffer)) { + MOZ_ASSERT(mallocedBufferBytes >= oldBytes); void* newBuffer = zone->pod_realloc((uint8_t*)oldBuffer, oldBytes, newBytes); - if (newBuffer && oldBuffer != newBuffer) { - MOZ_ALWAYS_TRUE(mallocedBuffers.rekeyAs(oldBuffer, newBuffer, newBuffer)); + if (newBuffer) { + if (oldBuffer != newBuffer) { + MOZ_ALWAYS_TRUE( + mallocedBuffers.rekeyAs(oldBuffer, newBuffer, newBuffer)); + } + mallocedBufferBytes -= oldBytes; + mallocedBufferBytes += newBytes; } return newBuffer; } @@ -662,9 +668,9 @@ void* js::Nursery::allocateBuffer(JS::BigInt* bi, size_t nbytes) { return allocateBuffer(bi->zone(), nbytes); } -void js::Nursery::freeBuffer(void* buffer) { +void js::Nursery::freeBuffer(void* buffer, size_t nbytes) { if (!isInside(buffer)) { - removeMallocedBuffer(buffer); + removeMallocedBuffer(buffer, nbytes); js_free(buffer); } } @@ -1123,6 +1129,7 @@ void js::Nursery::doCollection(JS::GCReason reason, // Sweep. startProfile(ProfileKey::FreeMallocedBuffers); gc->queueBuffersForFreeAfterMinorGC(mallocedBuffers); + mallocedBufferBytes = 0; endProfile(ProfileKey::FreeMallocedBuffers); startProfile(ProfileKey::ClearNursery); @@ -1260,9 +1267,19 @@ float js::Nursery::doPretenuring(JSRuntime* rt, JS::GCReason reason, return promotionRate; } -bool js::Nursery::registerMallocedBuffer(void* buffer) { +bool js::Nursery::registerMallocedBuffer(void* buffer, size_t nbytes) { MOZ_ASSERT(buffer); - return mallocedBuffers.putNew(buffer); + MOZ_ASSERT(nbytes > 0); + if (!mallocedBuffers.putNew(buffer)) { + return false; + } + + mallocedBufferBytes += nbytes; + if (MOZ_UNLIKELY(mallocedBufferBytes > capacity() * 8)) { + requestMinorGC(JS::GCReason::NURSERY_MALLOC_BUFFERS); + } + + return true; } void js::Nursery::sweep(JSTracer* trc) { diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index ca4a614cc6b3..1f8c1edb2260 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -317,7 +317,7 @@ class Nursery { void* allocateBuffer(JS::BigInt* bi, size_t nbytes); // Free an object buffer. - void freeBuffer(void* buffer); + void freeBuffer(void* buffer, size_t nbytes); // The maximum number of bytes allowed to reside in nursery buffers. static const size_t MaxNurseryBufferSize = 1024; @@ -342,10 +342,22 @@ class Nursery { // Register a malloced buffer that is held by a nursery object, which // should be freed at the end of a minor GC. Buffers are unregistered when // their owning objects are tenured. - MOZ_MUST_USE bool registerMallocedBuffer(void* buffer); + MOZ_MUST_USE bool registerMallocedBuffer(void* buffer, size_t nbytes); // Mark a malloced buffer as no longer needing to be freed. - void removeMallocedBuffer(void* buffer) { + void removeMallocedBuffer(void* buffer, size_t nbytes) { + MOZ_ASSERT(mallocedBuffers.has(buffer)); + MOZ_ASSERT(nbytes > 0); + MOZ_ASSERT(mallocedBufferBytes >= nbytes); + mallocedBuffers.remove(buffer); + mallocedBufferBytes -= nbytes; + } + + // Mark a malloced buffer as no longer needing to be freed during minor + // GC. There's no need to account for the size here since all remaining + // buffers will soon be freed. + void removeMallocedBufferDuringMinorGC(void* buffer) { + MOZ_ASSERT(JS::RuntimeHeapIsMinorCollecting()); MOZ_ASSERT(mallocedBuffers.has(buffer)); mallocedBuffers.remove(buffer); } @@ -542,6 +554,7 @@ class Nursery { // stored in the nursery. Any external buffers that do not belong to a // tenured thing at the end of a minor GC must be freed. BufferSet mallocedBuffers; + size_t mallocedBufferBytes = 0; // During a collection most hoisted slot and element buffers indicate their // new location with a forwarding pointer at the base. This does not work diff --git a/js/src/vm/ArgumentsObject.cpp b/js/src/vm/ArgumentsObject.cpp index a4e169b31085..1678bce1f687 100644 --- a/js/src/vm/ArgumentsObject.cpp +++ b/js/src/vm/ArgumentsObject.cpp @@ -939,7 +939,7 @@ size_t ArgumentsObject::objectMoved(JSObject* dst, JSObject* src) { size_t nbytesTotal = 0; uint32_t nDataBytes = ArgumentsData::bytesRequired(nsrc->data()->numArgs); if (!nursery.isInside(nsrc->data())) { - nursery.removeMallocedBuffer(nsrc->data()); + nursery.removeMallocedBufferDuringMinorGC(nsrc->data()); } else { AutoEnterOOMUnsafeRegion oomUnsafe; uint8_t* data = nsrc->zone()->pod_malloc(nDataBytes); @@ -959,7 +959,7 @@ size_t ArgumentsObject::objectMoved(JSObject* dst, JSObject* src) { if (RareArgumentsData* srcRareData = nsrc->maybeRareData()) { uint32_t nbytes = RareArgumentsData::bytesRequired(nsrc->initialLength()); if (!nursery.isInside(srcRareData)) { - nursery.removeMallocedBuffer(srcRareData); + nursery.removeMallocedBufferDuringMinorGC(srcRareData); } else { AutoEnterOOMUnsafeRegion oomUnsafe; uint8_t* dstRareData = nsrc->zone()->pod_malloc(nbytes); diff --git a/js/src/vm/BigIntType.cpp b/js/src/vm/BigIntType.cpp index bd100fbd1244..3bef1eec9f9e 100644 --- a/js/src/vm/BigIntType.cpp +++ b/js/src/vm/BigIntType.cpp @@ -1446,14 +1446,15 @@ JSLinearString* BigInt::toStringGeneric(JSContext* cx, HandleBigInt x, maximumCharactersRequired - writePos); } -static void FreeDigits(JSContext* cx, BigInt* bi, BigInt::Digit* digits) { +static void FreeDigits(JSContext* cx, BigInt* bi, BigInt::Digit* digits, + size_t nbytes) { if (cx->isHelperThreadContext()) { js_free(digits); } else if (bi->isTenured()) { MOZ_ASSERT(!cx->nursery().isInside(digits)); js_free(digits); } else { - cx->nursery().freeBuffer(digits); + cx->nursery().freeBuffer(digits, nbytes); } } @@ -1497,9 +1498,9 @@ BigInt* BigInt::destructivelyTrimHighZeroDigits(JSContext* cx, BigInt* x) { Digit digits[InlineDigitsLength]; std::copy_n(x->heapDigits_, InlineDigitsLength, digits); - FreeDigits(cx, x, x->heapDigits_); - RemoveCellMemory(x, x->digitLength() * sizeof(Digit), - js::MemoryUse::BigIntDigits); + size_t nbytes = x->digitLength() * sizeof(Digit); + FreeDigits(cx, x, x->heapDigits_, nbytes); + RemoveCellMemory(x, nbytes, js::MemoryUse::BigIntDigits); std::copy_n(digits, InlineDigitsLength, x->inlineDigits_); } diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index db16977261c5..4790b9bd1420 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -424,15 +424,15 @@ bool NativeObject::addDenseElementPure(JSContext* cx, NativeObject* obj) { return true; } -static inline void FreeSlots(JSContext* cx, NativeObject* obj, - HeapSlot* slots) { +static inline void FreeSlots(JSContext* cx, NativeObject* obj, HeapSlot* slots, + size_t nbytes) { if (cx->isHelperThreadContext()) { js_free(slots); } else if (obj->isTenured()) { MOZ_ASSERT(!cx->nursery().isInside(slots)); js_free(slots); } else { - cx->nursery().freeBuffer(slots); + cx->nursery().freeBuffer(slots, nbytes); } } @@ -441,9 +441,9 @@ void NativeObject::shrinkSlots(JSContext* cx, uint32_t oldCount, MOZ_ASSERT(newCount < oldCount); if (newCount == 0) { - RemoveCellMemory(this, numDynamicSlots() * sizeof(HeapSlot), - MemoryUse::ObjectSlots); - FreeSlots(cx, this, slots_); + size_t nbytes = numDynamicSlots() * sizeof(HeapSlot); + RemoveCellMemory(this, nbytes, MemoryUse::ObjectSlots); + FreeSlots(cx, this, slots_, nbytes); slots_ = nullptr; return; } diff --git a/js/src/vm/StringType-inl.h b/js/src/vm/StringType-inl.h index 41f7ce2050df..fcb04165a5cb 100644 --- a/js/src/vm/StringType-inl.h +++ b/js/src/vm/StringType-inl.h @@ -276,7 +276,8 @@ MOZ_ALWAYS_INLINE JSLinearString* JSLinearString::new_( // If the following registration fails, the string is partially initialized // and must be made valid, or its finalizer may attempt to free // uninitialized memory. - if (!cx->runtime()->gc.nursery().registerMallocedBuffer(chars.get())) { + if (!cx->runtime()->gc.nursery().registerMallocedBuffer( + chars.get(), length * sizeof(CharT))) { str->init(static_cast(nullptr), 0); if (allowGC) { ReportOutOfMemory(cx); diff --git a/js/src/vm/StringType.cpp b/js/src/vm/StringType.cpp index dc4e9f59c1b9..fa1f89675f82 100644 --- a/js/src/vm/StringType.cpp +++ b/js/src/vm/StringType.cpp @@ -652,7 +652,8 @@ JSLinearString* JSRope::flattenInternal(JSContext* maybecx) { if (!inTenured && left.isTenured()) { // tenured leftmost child is giving its chars buffer to the // nursery-allocated root node. - if (!nursery.registerMallocedBuffer(wholeChars)) { + if (!nursery.registerMallocedBuffer(wholeChars, + wholeCapacity * sizeof(CharT))) { if (maybecx) { ReportOutOfMemory(maybecx); } @@ -663,7 +664,7 @@ JSLinearString* JSRope::flattenInternal(JSContext* maybecx) { } else if (inTenured && !left.isTenured()) { // leftmost child is giving its nursery-held chars buffer to a // tenured string. - nursery.removeMallocedBuffer(wholeChars); + nursery.removeMallocedBuffer(wholeChars, wholeCapacity * sizeof(CharT)); } /* @@ -717,7 +718,8 @@ JSLinearString* JSRope::flattenInternal(JSContext* maybecx) { if (!isTenured()) { Nursery& nursery = runtimeFromMainThread()->gc.nursery(); - if (!nursery.registerMallocedBuffer(wholeChars)) { + if (!nursery.registerMallocedBuffer(wholeChars, + wholeCapacity * sizeof(CharT))) { js_free(wholeChars); if (maybecx) { ReportOutOfMemory(maybecx); diff --git a/js/src/vm/TypedArrayObject.cpp b/js/src/vm/TypedArrayObject.cpp index f134c1f80264..d9741fb37ca8 100644 --- a/js/src/vm/TypedArrayObject.cpp +++ b/js/src/vm/TypedArrayObject.cpp @@ -206,7 +206,7 @@ size_t TypedArrayObject::objectMoved(JSObject* obj, JSObject* old) { Nursery& nursery = obj->runtimeFromMainThread()->gc.nursery(); if (!nursery.isInside(buf)) { - nursery.removeMallocedBuffer(buf); + nursery.removeMallocedBufferDuringMinorGC(buf); size_t nbytes = RoundUp(newObj->byteLength(), sizeof(Value)); AddCellMemory(newObj, nbytes, MemoryUse::TypedArrayElements); return 0;