From 1527095aeaa81fdbbce8e7aeb218319bb640791f Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Tue, 23 Oct 2018 21:48:50 +0300 Subject: [PATCH] Backed out 4 changesets (bug 1489572) for valgrind failures at js::jit::LIRGenerator::visitBlock. Backed out changeset f3aa68c506e0 (bug 1489572) Backed out changeset cbfcb4b2b5c9 (bug 1489572) Backed out changeset 683e3e0eaee1 (bug 1489572) Backed out changeset f5973de3f6e8 (bug 1489572) --- js/src/ds/LifoAlloc.cpp | 242 ++----------------------------------- js/src/ds/LifoAlloc.h | 177 +++++++++++++++------------ js/src/frontend/Parser.cpp | 2 +- js/src/gc/StoreBuffer.h | 6 - 4 files changed, 115 insertions(+), 312 deletions(-) diff --git a/js/src/ds/LifoAlloc.cpp b/js/src/ds/LifoAlloc.cpp index ba865ac9942c..0a715a3ce4ea 100644 --- a/js/src/ds/LifoAlloc.cpp +++ b/js/src/ds/LifoAlloc.cpp @@ -27,6 +27,7 @@ namespace detail { UniquePtr BumpChunk::newWithCapacity(size_t size) { + MOZ_DIAGNOSTIC_ASSERT(RoundUpPow2(size) == size); MOZ_DIAGNOSTIC_ASSERT(size >= sizeof(BumpChunk)); void* mem = js_malloc(size); if (!mem) { @@ -113,17 +114,12 @@ LifoAlloc::reset(size_t defaultChunkSize) while (!chunks_.empty()) { chunks_.popFirst(); } - while (!oversize_.empty()) { - chunks_.popFirst(); - } while (!unused_.empty()) { unused_.popFirst(); } defaultChunkSize_ = defaultChunkSize; - oversizeThreshold_ = defaultChunkSize; markCount = 0; curSize_ = 0; - oversizeSize_ = 0; } void @@ -133,11 +129,6 @@ LifoAlloc::freeAll() UniqueBumpChunk bc = chunks_.popFirst(); decrementCurSize(bc->computedSizeOfIncludingThis()); } - while (!oversize_.empty()) { - UniqueBumpChunk bc = oversize_.popFirst(); - decrementCurSize(bc->computedSizeOfIncludingThis()); - oversizeSize_ -= bc->computedSizeOfIncludingThis(); - } while (!unused_.empty()) { UniqueBumpChunk bc = unused_.popFirst(); decrementCurSize(bc->computedSizeOfIncludingThis()); @@ -146,40 +137,10 @@ LifoAlloc::freeAll() // Nb: maintaining curSize_ correctly isn't easy. Fortunately, this is an // excellent sanity check. MOZ_ASSERT(curSize_ == 0); - MOZ_ASSERT(oversizeSize_ == 0); -} - -// Round at the same page granularity used by malloc. -static size_t -MallocGoodSize(size_t aSize) -{ -# if defined(MOZ_MEMORY) - return malloc_good_size(aSize); -# else - return aSize; -# endif -} - -// Heuristic to choose the size of the next BumpChunk for small allocations. -// `start` is the size of the first chunk. `used` is the total size of all -// BumpChunks in this LifoAlloc so far. -static size_t -NextSize(size_t start, size_t used) -{ - // Double the size, up to 1 MB. - const size_t mb = 1 * 1024 * 1024; - if (used < mb) { - return Max(start, used); - } - - // After 1 MB, grow more gradually, to waste less memory. - // The sequence (in megabytes) begins: - // 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 4, 4, 5, ... - return JS_ROUNDUP(used / 8, mb); } LifoAlloc::UniqueBumpChunk -LifoAlloc::newChunkWithCapacity(size_t n, bool oversize) +LifoAlloc::newChunkWithCapacity(size_t n) { MOZ_ASSERT(fallibleScope_, "[OOM] Cannot allocate a new chunk in an infallible scope."); @@ -193,10 +154,9 @@ LifoAlloc::newChunkWithCapacity(size_t n, bool oversize) return nullptr; } - MOZ_ASSERT(curSize_ >= oversizeSize_); - const size_t chunkSize = (oversize || minSize > defaultChunkSize_) - ? MallocGoodSize(minSize) - : NextSize(defaultChunkSize_, curSize_ - oversizeSize_); + const size_t chunkSize = minSize > defaultChunkSize_ + ? RoundUpPow2(minSize) + : defaultChunkSize_; // Create a new BumpChunk, and allocate space for it. UniqueBumpChunk result = detail::BumpChunk::newWithCapacity(chunkSize); @@ -207,7 +167,7 @@ LifoAlloc::newChunkWithCapacity(size_t n, bool oversize) return result; } -LifoAlloc::UniqueBumpChunk +bool LifoAlloc::getOrCreateChunk(size_t n) { // Look for existing unused BumpChunks to satisfy the request, and pick the @@ -215,7 +175,8 @@ LifoAlloc::getOrCreateChunk(size_t n) // chunks. if (!unused_.empty()) { if (unused_.begin()->canAlloc(n)) { - return unused_.popFirst(); + chunks_.append(unused_.popFirst()); + return true; } BumpChunkList::Iterator e(unused_.end()); @@ -224,170 +185,24 @@ LifoAlloc::getOrCreateChunk(size_t n) MOZ_ASSERT(elem->empty()); if (elem->canAlloc(n)) { BumpChunkList temp = unused_.splitAfter(i.get()); - UniqueBumpChunk newChunk = temp.popFirst(); + chunks_.append(temp.popFirst()); unused_.appendAll(std::move(temp)); - return newChunk; + return true; } } } // Allocate a new BumpChunk with enough space for the next allocation. - UniqueBumpChunk newChunk = newChunkWithCapacity(n, false); - if (!newChunk) { - return newChunk; - } - size_t size = newChunk->computedSizeOfIncludingThis(); - incrementCurSize(size); - return newChunk; -} - -void* -LifoAlloc::allocImplColdPath(size_t n) -{ - void* result; - UniqueBumpChunk newChunk = getOrCreateChunk(n); - if (!newChunk) { - return nullptr; - } - - // Since we just created a large enough chunk, this can't fail. - chunks_.append(std::move(newChunk)); - result = chunks_.last()->tryAlloc(n); - MOZ_ASSERT(result); - return result; -} - -void* -LifoAlloc::allocImplOversize(size_t n) -{ - void* result; - UniqueBumpChunk newChunk = newChunkWithCapacity(n, true); - if (!newChunk) { - return nullptr; - } - incrementCurSize(newChunk->computedSizeOfIncludingThis()); - oversizeSize_ += newChunk->computedSizeOfIncludingThis(); - - // Since we just created a large enough chunk, this can't fail. - oversize_.append(std::move(newChunk)); - result = oversize_.last()->tryAlloc(n); - MOZ_ASSERT(result); - return result; -} - -bool -LifoAlloc::ensureUnusedApproximateColdPath(size_t n, size_t total) -{ - for (detail::BumpChunk& bc : unused_) { - total += bc.unused(); - if (total >= n) { - return true; - } - } - - UniqueBumpChunk newChunk = newChunkWithCapacity(n, false); + UniqueBumpChunk newChunk = newChunkWithCapacity(n); if (!newChunk) { return false; } size_t size = newChunk->computedSizeOfIncludingThis(); - unused_.pushFront(std::move(newChunk)); + chunks_.append(std::move(newChunk)); incrementCurSize(size); return true; } -LifoAlloc::Mark -LifoAlloc::mark() -{ - markCount++; - Mark res; - if (!chunks_.empty()) { - res.chunk = chunks_.last()->mark(); - } - if (!oversize_.empty()) { - res.oversize = oversize_.last()->mark(); - } - return res; -} - -void -LifoAlloc::release(Mark mark) -{ - markCount--; -#ifdef DEBUG - auto assertIsContained = [](const detail::BumpChunk::Mark& m, BumpChunkList& list) { - if (m.markedChunk()) { - bool contained = false; - for (const detail::BumpChunk& chunk : list) { - if (&chunk == m.markedChunk() && chunk.contains(m)) { - contained = true; - break; - } - } - MOZ_ASSERT(contained); - } - }; - assertIsContained(mark.chunk, chunks_); - assertIsContained(mark.oversize, oversize_); -#endif - - BumpChunkList released; - auto cutAtMark = [&released](const detail::BumpChunk::Mark& m, BumpChunkList& list) { - // Move the blocks which are after the mark to the set released chunks. - if (!m.markedChunk()) { - released = std::move(list); - } else { - released = list.splitAfter(m.markedChunk()); - } - - // Release everything which follows the mark in the last chunk. - if (!list.empty()) { - list.last()->release(m); - } - }; - - // Release the content of all the blocks which are after the marks, and keep - // blocks as unused. - cutAtMark(mark.chunk, chunks_); - for (detail::BumpChunk& bc : released) { - bc.release(); - } - unused_.appendAll(std::move(released)); - - // Free the content of all the blocks which are after the marks. - cutAtMark(mark.oversize, oversize_); - while (!released.empty()) { - UniqueBumpChunk bc = released.popFirst(); - decrementCurSize(bc->computedSizeOfIncludingThis()); - oversizeSize_ -= bc->computedSizeOfIncludingThis(); - } -} - -void -LifoAlloc::steal(LifoAlloc* other) -{ - MOZ_ASSERT(!other->markCount); - MOZ_DIAGNOSTIC_ASSERT(unused_.empty()); - MOZ_DIAGNOSTIC_ASSERT(chunks_.empty()); - MOZ_DIAGNOSTIC_ASSERT(oversize_.empty()); - - // Copy everything from |other| to |this| except for |peakSize_|, which - // requires some care. - chunks_ = std::move(other->chunks_); - oversize_ = std::move(other->oversize_); - unused_ = std::move(other->unused_); - markCount = other->markCount; - defaultChunkSize_ = other->defaultChunkSize_; - oversizeThreshold_ = other->oversizeThreshold_; - curSize_ = other->curSize_; - peakSize_ = Max(peakSize_, other->peakSize_); - oversizeSize_ = other->oversizeSize_; -#if defined(DEBUG) || defined(JS_OOM_BREAKPOINT) - fallibleScope_ = other->fallibleScope_; -#endif - - other->reset(defaultChunkSize_); -} - void LifoAlloc::transferFrom(LifoAlloc* other) { @@ -395,12 +210,9 @@ LifoAlloc::transferFrom(LifoAlloc* other) MOZ_ASSERT(!other->markCount); incrementCurSize(other->curSize_); - oversizeSize_ += other->oversizeSize_; appendUnused(std::move(other->unused_)); appendUsed(std::move(other->chunks_)); - oversize_.appendAll(std::move(other->oversize_)); other->curSize_ = 0; - other->oversizeSize_ = 0; } void @@ -417,33 +229,3 @@ LifoAlloc::transferUnusedFrom(LifoAlloc* other) incrementCurSize(size); other->decrementCurSize(size); } - -#ifdef LIFO_CHUNK_PROTECT -void -LifoAlloc::setReadOnly() -{ - for (detail::BumpChunk& bc : chunks_) { - bc.setReadOnly(); - } - for (detail::BumpChunk& bc : oversize_) { - bc.setReadOnly(); - } - for (detail::BumpChunk& bc : unused_) { - bc.setReadOnly(); - } -} - -void -LifoAlloc::setReadWrite() -{ - for (detail::BumpChunk& bc : chunks_) { - bc.setReadWrite(); - } - for (detail::BumpChunk& bc : oversize_) { - bc.setReadWrite(); - } - for (detail::BumpChunk& bc : unused_) { - bc.setReadWrite(); - } -} -#endif diff --git a/js/src/ds/LifoAlloc.h b/js/src/ds/LifoAlloc.h index 1cde6a1534f1..1d1b0c8d5824 100644 --- a/js/src/ds/LifoAlloc.h +++ b/js/src/ds/LifoAlloc.h @@ -379,7 +379,7 @@ class BumpChunk : public SingleLinkedListElement uint8_t* bump_; friend class BumpChunk; - Mark(BumpChunk* chunk, uint8_t* bump) + explicit Mark(BumpChunk* chunk, uint8_t* bump) : chunk_(chunk), bump_(bump) {} @@ -527,29 +527,19 @@ class LifoAlloc using UniqueBumpChunk = js::UniquePtr; using BumpChunkList = detail::SingleLinkedList; - // List of chunks containing allocated data of size smaller than the default - // chunk size. In the common case, the last chunk of this list is always - // used to perform the allocations. When the allocation cannot be performed, - // we move a Chunk from the unused set to the list of used chunks. + // List of chunks containing allocated data. In the common case, the last + // chunk of this list is always used to perform the allocations. When the + // allocation cannot be performed, we move a Chunk from the unused set to + // the list of used chunks. BumpChunkList chunks_; - // List of chunks containing allocated data where each allocation is larger - // than the oversize threshold. Each chunk contains exactly on allocation. - // This reduces wasted space in the normal chunk list. - // - // Oversize chunks are allocated on demand and freed as soon as they are - // released, instead of being pushed to the unused list. - BumpChunkList oversize_; - // Set of unused chunks, which can be reused for future allocations. BumpChunkList unused_; size_t markCount; size_t defaultChunkSize_; - size_t oversizeThreshold_; size_t curSize_; size_t peakSize_; - size_t oversizeSize_; #if defined(DEBUG) || defined(JS_OOM_BREAKPOINT) bool fallibleScope_; #endif @@ -558,11 +548,11 @@ class LifoAlloc LifoAlloc(const LifoAlloc&) = delete; // Return a BumpChunk that can perform an allocation of at least size |n|. - UniqueBumpChunk newChunkWithCapacity(size_t n, bool oversize); + UniqueBumpChunk newChunkWithCapacity(size_t n); // Reuse or allocate a BumpChunk that can perform an allocation of at least // size |n|, if successful it is placed at the end the list of |chunks_|. - UniqueBumpChunk getOrCreateChunk(size_t n); + MOZ_MUST_USE bool getOrCreateChunk(size_t n); void reset(size_t defaultChunkSize); @@ -594,25 +584,22 @@ class LifoAlloc curSize_ -= size; } - void* allocImplColdPath(size_t n); - void* allocImplOversize(size_t n); - MOZ_ALWAYS_INLINE void* allocImpl(size_t n) { void* result; - // Give oversized allocations their own chunk instead of wasting space - // due to fragmentation at the end of normal chunk. - if (MOZ_UNLIKELY(n > oversizeThreshold_)) { - return allocImplOversize(n); - } - if (MOZ_LIKELY(!chunks_.empty() && (result = chunks_.last()->tryAlloc(n)))) { + if (!chunks_.empty() && (result = chunks_.last()->tryAlloc(n))) { return result; } - return allocImplColdPath(n); - } - // Check for space in unused chunks or allocate a new unused chunk. - MOZ_MUST_USE bool ensureUnusedApproximateColdPath(size_t n, size_t total); + if (!getOrCreateChunk(n)) { + return nullptr; + } + + // Since we just created a large enough chunk, this can't fail. + result = chunks_.last()->tryAlloc(n); + MOZ_ASSERT(result); + return result; + } public: explicit LifoAlloc(size_t defaultChunkSize) @@ -624,18 +611,26 @@ class LifoAlloc reset(defaultChunkSize); } - // Set the threshold to allocate data in its own chunk outside the space for - // small allocations. - void disableOversize() { - oversizeThreshold_ = SIZE_MAX; - } - void setOversizeThreshold(size_t oversizeThreshold) { - MOZ_ASSERT(oversizeThreshold <= defaultChunkSize_); - oversizeThreshold_ = oversizeThreshold; - } - // Steal allocated chunks from |other|. - void steal(LifoAlloc* other); + void steal(LifoAlloc* other) { + MOZ_ASSERT(!other->markCount); + MOZ_DIAGNOSTIC_ASSERT(unused_.empty()); + MOZ_DIAGNOSTIC_ASSERT(chunks_.empty()); + + // Copy everything from |other| to |this| except for |peakSize_|, which + // requires some care. + chunks_ = std::move(other->chunks_); + unused_ = std::move(other->unused_); + markCount = other->markCount; + defaultChunkSize_ = other->defaultChunkSize_; + curSize_ = other->curSize_; + peakSize_ = Max(peakSize_, other->peakSize_); +#if defined(DEBUG) || defined(JS_OOM_BREAKPOINT) + fallibleScope_ = other->fallibleScope_; +#endif + + other->reset(defaultChunkSize_); + } // Append all chunks from |other|. They are removed from |other|. void transferFrom(LifoAlloc* other); @@ -671,7 +666,7 @@ class LifoAlloc template MOZ_ALWAYS_INLINE T* - newWithSize(size_t n, Args&&... args) + allocInSize(size_t n, Args&&... args) { MOZ_ASSERT(n >= sizeof(T), "must request enough space to store a T"); static_assert(alignof(T) <= detail::LIFO_ALLOC_ALIGN, @@ -708,7 +703,21 @@ class LifoAlloc } } - return ensureUnusedApproximateColdPath(n, total); + for (detail::BumpChunk& bc : unused_) { + total += bc.unused(); + if (total >= n) { + return true; + } + } + + UniqueBumpChunk newChunk = newChunkWithCapacity(n); + if (!newChunk) { + return false; + } + size_t size = newChunk->computedSizeOfIncludingThis(); + unused_.pushFront(std::move(newChunk)); + incrementCurSize(size); + return true; } MOZ_ALWAYS_INLINE @@ -760,13 +769,38 @@ class LifoAlloc return static_cast(alloc(bytes)); } - class Mark { - friend class LifoAlloc; - detail::BumpChunk::Mark chunk; - detail::BumpChunk::Mark oversize; - }; - Mark mark(); - void release(Mark mark); + using Mark = detail::BumpChunk::Mark; + + Mark mark() { + markCount++; + if (chunks_.empty()) { + return Mark(); + } + return chunks_.last()->mark(); + } + + void release(Mark mark) { + markCount--; + + // Move the blocks which are after the mark to the set of unused chunks. + BumpChunkList released; + if (!mark.markedChunk()) { + released = std::move(chunks_); + } else { + released = chunks_.splitAfter(mark.markedChunk()); + } + + // Release the content of all the blocks which are after the marks. + for (detail::BumpChunk& bc : released) { + bc.release(); + } + unused_.appendAll(std::move(released)); + + // Release everything which follows the mark in the last chunk. + if (!chunks_.empty()) { + chunks_.last()->release(mark); + } + } void releaseAll() { MOZ_ASSERT(!markCount); @@ -774,19 +808,26 @@ class LifoAlloc bc.release(); } unused_.appendAll(std::move(chunks_)); - // On release, we free any oversize allocations instead of keeping them - // in unused chunks. - while (!oversize_.empty()) { - UniqueBumpChunk bc = oversize_.popFirst(); - decrementCurSize(bc->computedSizeOfIncludingThis()); - oversizeSize_ -= bc->computedSizeOfIncludingThis(); - } } // Protect the content of the LifoAlloc chunks. #ifdef LIFO_CHUNK_PROTECT - void setReadOnly(); - void setReadWrite(); + void setReadOnly() { + for (detail::BumpChunk& bc : chunks_) { + bc.setReadOnly(); + } + for (detail::BumpChunk& bc : unused_) { + bc.setReadOnly(); + } + } + void setReadWrite() { + for (detail::BumpChunk& bc : chunks_) { + bc.setReadWrite(); + } + for (detail::BumpChunk& bc : unused_) { + bc.setReadWrite(); + } + } #else void setReadOnly() const {} void setReadWrite() const {} @@ -803,10 +844,8 @@ class LifoAlloc // Return true if the LifoAlloc does not currently contain any allocations. bool isEmpty() const { - bool empty = chunks_.empty() || - (chunks_.begin() == chunks_.last() && chunks_.last()->empty()); - MOZ_ASSERT_IF(!oversize_.empty(), !oversize_.last()->empty()); - return empty && oversize_.empty(); + return chunks_.empty() || + (chunks_.begin() == chunks_.last() && chunks_.last()->empty()); } // Return the number of bytes remaining to allocate in the current chunk. @@ -824,9 +863,6 @@ class LifoAlloc for (const detail::BumpChunk& chunk : chunks_) { n += chunk.sizeOfIncludingThis(mallocSizeOf); } - for (const detail::BumpChunk& chunk : oversize_) { - n += chunk.sizeOfIncludingThis(mallocSizeOf); - } for (const detail::BumpChunk& chunk : unused_) { n += chunk.sizeOfIncludingThis(mallocSizeOf); } @@ -839,9 +875,6 @@ class LifoAlloc for (const detail::BumpChunk& chunk : chunks_) { n += chunk.computedSizeOfIncludingThis(); } - for (const detail::BumpChunk& chunk : oversize_) { - n += chunk.computedSizeOfIncludingThis(); - } for (const detail::BumpChunk& chunk : unused_) { n += chunk.computedSizeOfIncludingThis(); } @@ -874,11 +907,6 @@ class LifoAlloc return true; } } - for (const detail::BumpChunk& chunk : oversize_) { - if (chunk.contains(ptr)) { - return true; - } - } return false; } #endif @@ -919,7 +947,6 @@ class LifoAlloc chunkEnd_(alloc.chunks_.end()), head_(nullptr) { - MOZ_RELEASE_ASSERT(alloc.oversize_.empty()); if (chunkIt_ != chunkEnd_) { head_ = chunkIt_->begin(); } diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 943a7c99ff7f..1a9a98251782 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -1938,7 +1938,7 @@ NewEmptyBindingData(JSContext* cx, LifoAlloc& alloc, uint32_t numBindings) { using Data = typename Scope::Data; size_t allocSize = SizeOfData(numBindings); - auto* bindings = alloc.newWithSize(allocSize, numBindings); + auto* bindings = alloc.allocInSize(allocSize, numBindings); if (!bindings) { ReportOutOfMemory(cx); } diff --git a/js/src/gc/StoreBuffer.h b/js/src/gc/StoreBuffer.h index af1b5a2188a2..269c6ffeb51d 100644 --- a/js/src/gc/StoreBuffer.h +++ b/js/src/gc/StoreBuffer.h @@ -153,12 +153,6 @@ class StoreBuffer MOZ_ASSERT(!head_); if (!storage_) { storage_ = js_new(LifoAllocBlockSize); - // This prevents LifoAlloc::Enum from crashing with a release - // assertion if we ever allocate one entry larger than - // LifoAllocBlockSize. - if (storage_) { - storage_->disableOversize(); - } } clear(); return bool(storage_);