Attachment 361811 Details for Bug 194547 – ROLLOUT of r241182

[patch] ROLLOUT of r241182

bug-194547-20190212111746.patch (text/plain), 10.95 KB, created by WebKit Commit Bot on 2019-02-12 11:17:47 PST

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2019-02-12 11:17:47 PST

Size: 10.95 KB

patch

obsolete

>Subversion Revision: 241304
>diff --git a/Source/bmalloc/ChangeLog b/Source/bmalloc/ChangeLog
>index 10800e3ce06b1475a2662721b6837dfcbad30687..88b98ed3685390074762a189fa2fc4e1d43819d4 100644
>--- a/Source/bmalloc/ChangeLog
>+++ b/Source/bmalloc/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-02-12  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r241182.
>+        https://bugs.webkit.org/show_bug.cgi?id=194547
>+
>+        causes a 2-3% Speedometer2 regression. (Requested by
>+        keith_miller on #webkit).
>+
>+        Reverted changeset:
>+
>+        "bmalloc uses more memory on iOS compared to macOS due to
>+        physical page size differences"
>+        https://bugs.webkit.org/show_bug.cgi?id=192389
>+        https://trac.webkit.org/changeset/241182
>+
> 2019-02-07  Michael Saboff  <msaboff@apple.com>
> 
>         bmalloc uses more memory on iOS compared to macOS due to physical page size differences
>diff --git a/Source/bmalloc/bmalloc/Chunk.h b/Source/bmalloc/bmalloc/Chunk.h
>index 775dc17dd68882daa26e1c0d526dfa1acf4e5a96..31db6f2d7bf8b9ebe13057f2614a0fb8ef542639 100644
>--- a/Source/bmalloc/bmalloc/Chunk.h
>+++ b/Source/bmalloc/bmalloc/Chunk.h
>@@ -50,7 +50,6 @@ public:
>     char* address(size_t offset);
>     SmallPage* page(size_t offset);
>     SmallLine* line(size_t offset);
>-    size_t pageNumber(SmallPage*);
> 
>     char* bytes() { return reinterpret_cast<char*>(this); }
>     SmallLine* lines() { return &m_lines[0]; }
>@@ -78,8 +77,7 @@ template<typename Function> void forEachPage(Chunk* chunk, size_t pageSize, Func
> {
>     // We align to at least the page size so we can service aligned allocations
>     // at equal and smaller powers of two, and also so we can vmDeallocatePhysicalPages().
>-    size_t firstPageOffset = max(pageSize, vmPageSize());
>-    size_t metadataSize = roundUpToMultipleOfNonPowerOfTwo(firstPageOffset, sizeof(Chunk));
>+    size_t metadataSize = roundUpToMultipleOfNonPowerOfTwo(pageSize, sizeof(Chunk));
> 
>     Object begin(chunk, metadataSize);
>     Object end(chunk, chunkSize);
>diff --git a/Source/bmalloc/bmalloc/Heap.cpp b/Source/bmalloc/bmalloc/Heap.cpp
>index 5f7b93bbc8239ec1f0e364b057f69dc56f4fd6a9..528cfebf973d23d8c3453e279396b22da726cc0c 100644
>--- a/Source/bmalloc/bmalloc/Heap.cpp
>+++ b/Source/bmalloc/bmalloc/Heap.cpp
>@@ -44,8 +44,6 @@
> 
> namespace bmalloc {
> 
>-static_assert(isPowerOfTwo(smallPageSize), "");
>-
> Heap::Heap(HeapKind kind, std::lock_guard<Mutex>&)
>     : m_kind(kind)
>     , m_vmPageSizePhysical(vmPageSizePhysical())
>@@ -95,7 +93,7 @@ size_t Heap::gigacageSize()
> void Heap::initializeLineMetadata()
> {
>     size_t sizeClassCount = bmalloc::sizeClass(smallLineSize);
>-    size_t smallLineCount = smallPageSize / smallLineSize;
>+    size_t smallLineCount = m_vmPageSizePhysical / smallLineSize;
>     m_smallLineMetadata.grow(sizeClassCount * smallLineCount);
> 
>     for (size_t sizeClass = 0; sizeClass < sizeClassCount; ++sizeClass) {
>@@ -104,7 +102,7 @@ void Heap::initializeLineMetadata()
> 
>         size_t object = 0;
>         size_t line = 0;
>-        while (object < smallPageSize) {
>+        while (object < m_vmPageSizePhysical) {
>             line = object / smallLineSize;
>             size_t leftover = object % smallLineSize;
> 
>@@ -118,7 +116,7 @@ void Heap::initializeLineMetadata()
>         }
> 
>         // Don't allow the last object in a page to escape the page.
>-        if (object > smallPageSize) {
>+        if (object > m_vmPageSizePhysical) {
>             BASSERT(pageMetadata[line].objectCount);
>             --pageMetadata[line].objectCount;
>         }
>@@ -130,18 +128,11 @@ void Heap::initializePageMetadata()
>     auto computePageSize = [&](size_t sizeClass) {
>         size_t size = objectSize(sizeClass);
>         if (sizeClass < bmalloc::sizeClass(smallLineSize))
>-            return smallPageSize;
>+            return m_vmPageSizePhysical;
> 
>-        // We want power of 2 pageSizes sizes below physical page size and multiples of physical pages size above that.
>-        size_t pageSize = smallPageSize;
>-        for (; pageSize < m_vmPageSizePhysical; pageSize *= 2) {
>-            RELEASE_BASSERT(pageSize <= chunkSize / 2);
>-            size_t waste = pageSize % size;
>-            if (waste <= pageSize / pageSizeWasteFactor)
>-                return pageSize;
>-        }
>-
>-        for (; pageSize < pageSizeMax; pageSize += m_vmPageSizePhysical) {
>+        for (size_t pageSize = m_vmPageSizePhysical;
>+            pageSize < pageSizeMax;
>+            pageSize += m_vmPageSizePhysical) {
>             RELEASE_BASSERT(pageSize <= chunkSize / 2);
>             size_t waste = pageSize % size;
>             if (waste <= pageSize / pageSizeWasteFactor)
>@@ -197,17 +188,14 @@ void Heap::scavenge(std::lock_guard<Mutex>& lock, BulkDecommit& decommitter)
>                     continue;
> 
>                 size_t pageSize = bmalloc::pageSize(&list - &m_freePages[0]);
>-                if (pageSize >= m_vmPageSizePhysical) {
>-                    size_t decommitSize = physicalPageSizeSloppy(page->begin()->begin(), pageSize);
>-                    m_freeableMemory -= decommitSize;
>-                    m_footprint -= decommitSize;
>-                    decommitter.addEager(page->begin()->begin(), pageSize);
>-                    page->setHasPhysicalPages(false);
>-#if ENABLE_PHYSICAL_PAGE_MAP
>-                    m_physicalPageMap.decommit(page->begin()->begin(), pageSize);
>+                size_t decommitSize = physicalPageSizeSloppy(page->begin()->begin(), pageSize);
>+                m_freeableMemory -= decommitSize;
>+                m_footprint -= decommitSize;
>+                decommitter.addEager(page->begin()->begin(), pageSize);
>+                page->setHasPhysicalPages(false);
>+#if ENABLE_PHYSICAL_PAGE_MAP 
>+                m_physicalPageMap.decommit(page->begin()->begin(), pageSize);
> #endif
>-                } else
>-                    tryDecommitSmallPagesInPhysicalPage(lock, decommitter, page, pageSize);
>             }
>         }
>     }
>@@ -279,63 +267,6 @@ void Heap::allocateSmallChunk(std::unique_lock<Mutex>& lock, size_t pageClass)
>     m_freePages[pageClass].push(chunk);
> }
> 
>-void Heap::tryDecommitSmallPagesInPhysicalPage(std::lock_guard<Mutex>& lock, BulkDecommit& decommitter, SmallPage* smallPage, size_t pageSize)
>-{
>-    Chunk* chunk = Chunk::get(smallPage);
>-
>-    char* pageBegin = smallPage->begin()->begin();
>-    char* physicalPageBegin = roundDownToMultipleOf(m_vmPageSizePhysical, pageBegin);
>-
>-    // The first page in a physical page takes care of decommitting its physical neighbors
>-    if (pageBegin != physicalPageBegin)
>-        return;
>-
>-    size_t beginPageOffset = chunk->offset(physicalPageBegin);
>-    size_t endPageOffset = beginPageOffset + m_vmPageSizePhysical;
>-
>-    Object begin(chunk, beginPageOffset);
>-    Object end(chunk, endPageOffset);
>-
>-    for (auto it = begin; it + pageSize <= end; it = it + pageSize) {
>-        if (it.page()->refCount(lock))
>-            return;
>-    }
>-
>-    size_t decommitSize = m_vmPageSizePhysical;
>-    m_freeableMemory -= decommitSize;
>-    m_footprint -= decommitSize;
>-
>-    decommitter.addEager(physicalPageBegin, decommitSize);
>-
>-    for (auto it = begin; it + pageSize <= end; it = it + pageSize)
>-        it.page()->setHasPhysicalPages(false);
>-#if ENABLE_PHYSICAL_PAGE_MAP
>-    m_physicalPageMap.decommit(smallPage, decommitSize);
>-#endif
>-}
>-
>-void Heap::commitSmallPagesInPhysicalPage(std::unique_lock<Mutex>&, SmallPage* page, size_t pageSize)
>-{
>-    Chunk* chunk = Chunk::get(page);
>-
>-    char* physicalPageBegin = roundDownToMultipleOf(m_vmPageSizePhysical, page->begin()->begin());
>-
>-    size_t beginPageOffset = chunk->offset(physicalPageBegin);
>-    size_t endPageOffset = beginPageOffset + m_vmPageSizePhysical;
>-
>-    Object begin(chunk, beginPageOffset);
>-    Object end(chunk, endPageOffset);
>-
>-    m_footprint += m_vmPageSizePhysical;
>-    vmAllocatePhysicalPagesSloppy(physicalPageBegin, m_vmPageSizePhysical);
>-
>-    for (auto it = begin; it + pageSize <= end; it = it + pageSize)
>-        it.page()->setHasPhysicalPages(true);
>-#if ENABLE_PHYSICAL_PAGE_MAP
>-    m_physicalPageMap.commit(begin.page(), m_vmPageSizePhysical);
>-#endif
>-}
>-
> void Heap::deallocateSmallChunk(Chunk* chunk, size_t pageClass)
> {
>     m_objectTypes.set(chunk, ObjectType::Large);
>@@ -394,15 +325,12 @@ SmallPage* Heap::allocateSmallPage(std::unique_lock<Mutex>& lock, size_t sizeCla
>             m_freeableMemory -= physicalSize;
>         else {
>             m_scavenger->scheduleIfUnderMemoryPressure(pageSize);
>-            if (pageSize >= m_vmPageSizePhysical) {
>-                m_footprint += physicalSize;
>-                vmAllocatePhysicalPagesSloppy(page->begin()->begin(), pageSize);
>-                page->setHasPhysicalPages(true);
>-#if ENABLE_PHYSICAL_PAGE_MAP
>-                m_physicalPageMap.commit(page->begin()->begin(), pageSize);
>+            m_footprint += physicalSize;
>+            vmAllocatePhysicalPagesSloppy(page->begin()->begin(), pageSize);
>+            page->setHasPhysicalPages(true);
>+#if ENABLE_PHYSICAL_PAGE_MAP 
>+            m_physicalPageMap.commit(page->begin()->begin(), pageSize);
> #endif
>-            } else
>-                commitSmallPagesInPhysicalPage(lock, page, pageSize);
>         }
> 
>         return page;
>@@ -462,7 +390,7 @@ void Heap::allocateSmallBumpRangesByMetadata(
>     SmallPage* page = allocateSmallPage(lock, sizeClass, lineCache);
>     SmallLine* lines = page->begin();
>     BASSERT(page->hasFreeLines(lock));
>-    size_t smallLineCount = smallPageSize / smallLineSize;
>+    size_t smallLineCount = m_vmPageSizePhysical / smallLineSize;
>     LineMetadata* pageMetadata = &m_smallLineMetadata[sizeClass * smallLineCount];
>     
>     auto findSmallBumpRange = [&](size_t& lineNumber) {
>diff --git a/Source/bmalloc/bmalloc/Heap.h b/Source/bmalloc/bmalloc/Heap.h
>index 9025e5f18f6fd3a54a7b7efc0f17427b712c8777..7146dd7b1d7a5788b89e7d124fb1b473e14f189f 100644
>--- a/Source/bmalloc/bmalloc/Heap.h
>+++ b/Source/bmalloc/bmalloc/Heap.h
>@@ -120,9 +120,6 @@ private:
>     SmallPage* allocateSmallPage(std::unique_lock<Mutex>&, size_t sizeClass, LineCache&);
>     void deallocateSmallLine(std::unique_lock<Mutex>&, Object, LineCache&);
> 
>-    void tryDecommitSmallPagesInPhysicalPage(std::lock_guard<Mutex>&, BulkDecommit& decommitter, SmallPage*, size_t pageSize);
>-    void commitSmallPagesInPhysicalPage(std::unique_lock<Mutex>&, SmallPage*, size_t pageSize);
>-
>     void allocateSmallChunk(std::unique_lock<Mutex>&, size_t pageClass);
>     void deallocateSmallChunk(Chunk*, size_t pageClass);
> 
>diff --git a/Source/bmalloc/bmalloc/SmallPage.h b/Source/bmalloc/bmalloc/SmallPage.h
>index 933b1ef955f126a4198220f52842af304f767f5f..e024c8dc3a5bab482c0051e650136d155305db84 100644
>--- a/Source/bmalloc/bmalloc/SmallPage.h
>+++ b/Source/bmalloc/bmalloc/SmallPage.h
>@@ -41,8 +41,7 @@ public:
>     void ref(std::unique_lock<Mutex>&);
>     bool deref(std::unique_lock<Mutex>&);
>     unsigned refCount(std::unique_lock<Mutex>&) { return m_refCount; }
>-    unsigned refCount(std::lock_guard<Mutex>&) { return m_refCount; }
>-
>+    
>     size_t sizeClass() { return m_sizeClass; }
>     void setSizeClass(size_t sizeClass) { m_sizeClass = sizeClass; }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 194547: 361811