Attachment 362459 Details for Bug 194811 – Patch

[patch] Patch

bug-194811-20190219172447.patch (text/plain), 12.55 KB, created by Yusuke Suzuki on 2019-02-19 17:24:48 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-19 17:24:48 PST

Size: 12.55 KB

patch

obsolete

>Subversion Revision: 241769
>diff --git a/Source/bmalloc/ChangeLog b/Source/bmalloc/ChangeLog
>index 7ce650abaa61f19c916af47af18912ac66a94657..9a46d401b5a1e5973665ac3dece7d0a8b99ccfd6 100644
>--- a/Source/bmalloc/ChangeLog
>+++ b/Source/bmalloc/ChangeLog
>@@ -1,3 +1,46 @@
>+2019-02-19  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [bmalloc] bmalloc::Cache should not be instantiated if we are using system malloc
>+        https://bugs.webkit.org/show_bug.cgi?id=194811
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        bmalloc::Cache is very large. It is 13KB. Since it exists per HeapKind, it takes 40KB.
>+        But this is meaningless if we are under the system malloc mode by using "Malloc=1". We
>+        found that it continues using so much dirty memory region even under the system malloc mode.
>+        This patch avoids instantiation of bmalloc::Cache under the system malloc mode.
>+
>+        * bmalloc/Allocator.cpp:
>+        (bmalloc::Allocator::Allocator):
>+        (bmalloc::Allocator::tryAllocate):
>+        (bmalloc::Allocator::allocateImpl):
>+        (bmalloc::Allocator::reallocateImpl):
>+        (bmalloc::Allocator::allocateSlowCase):
>+        Allocator is a per Cache object. So we no longer need to keep m_debugHeap. If debug heap is enabled,
>+        Allocator is never created.
>+
>+        * bmalloc/Allocator.h:
>+        * bmalloc/Cache.cpp:
>+        (bmalloc::debugHeap):
>+        (bmalloc::Cache::Cache):
>+        (bmalloc::Cache::tryAllocateSlowCaseNullCache):
>+        (bmalloc::Cache::allocateSlowCaseNullCache):
>+        (bmalloc::Cache::deallocateSlowCaseNullCache):
>+        (bmalloc::Cache::tryReallocateSlowCaseNullCache):
>+        (bmalloc::Cache::reallocateSlowCaseNullCache):
>+        * bmalloc/Cache.h:
>+        (bmalloc::Cache::tryAllocate):
>+        (bmalloc::Cache::tryReallocate):
>+        If the debug heap mode is enabled, we keep Cache::getFast() returning nullptr. And in the slow path case, we use debugHeap.
>+        This makes bmalloc fast path fast, while we avoid Cache instantiation.
>+
>+        * bmalloc/Deallocator.cpp:
>+        (bmalloc::Deallocator::Deallocator):
>+        (bmalloc::Deallocator::scavenge):
>+        (bmalloc::Deallocator::deallocateSlowCase):
>+        * bmalloc/Deallocator.h:
>+        Ditto for Deallocator.
>+
> 2019-02-15  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [bmalloc] NSBundle-based application name check should be executed after debug-heap environment variable check
>diff --git a/Source/bmalloc/bmalloc/Allocator.cpp b/Source/bmalloc/bmalloc/Allocator.cpp
>index 4fbdb0964ab98ac6bce3c206181c9e7a31477a96..cfd01fe3ed6fda8f443172394f448195f23169ba 100644
>--- a/Source/bmalloc/bmalloc/Allocator.cpp
>+++ b/Source/bmalloc/bmalloc/Allocator.cpp
>@@ -27,7 +27,7 @@
> #include "BAssert.h"
> #include "Chunk.h"
> #include "Deallocator.h"
>-#include "DebugHeap.h"
>+#include "Environment.h"
> #include "Heap.h"
> #include "PerProcess.h"
> #include "Sizes.h"
>@@ -38,9 +38,9 @@ namespace bmalloc {
> 
> Allocator::Allocator(Heap& heap, Deallocator& deallocator)
>     : m_heap(heap)
>-    , m_debugHeap(heap.debugHeap())
>     , m_deallocator(deallocator)
> {
>+    BASSERT(!PerProcess<Environment>::get()->isDebugHeapEnabled());
>     for (size_t sizeClass = 0; sizeClass < sizeClassCount; ++sizeClass)
>         m_bumpAllocators[sizeClass].init(objectSize(sizeClass));
> }
>@@ -52,9 +52,6 @@ Allocator::~Allocator()
> 
> void* Allocator::tryAllocate(size_t size)
> {
>-    if (m_debugHeap)
>-        return m_debugHeap->malloc(size);
>-
>     if (size <= smallMax)
>         return allocate(size);
> 
>@@ -78,9 +75,6 @@ void* Allocator::allocateImpl(size_t alignment, size_t size, bool crashOnFailure
> {
>     BASSERT(isPowerOfTwo(alignment));
> 
>-    if (m_debugHeap)
>-        return m_debugHeap->memalign(alignment, size, crashOnFailure);
>-
>     if (!size)
>         size = alignment;
> 
>@@ -107,9 +101,6 @@ void* Allocator::tryReallocate(void* object, size_t newSize)
> 
> void* Allocator::reallocateImpl(void* object, size_t newSize, bool crashOnFailure)
> {
>-    if (m_debugHeap)
>-        return m_debugHeap->realloc(object, newSize, crashOnFailure);
>-
>     size_t oldSize = 0;
>     switch (objectType(m_heap.kind(), object)) {
>     case ObjectType::Small: {
>@@ -200,9 +191,6 @@ BNO_INLINE void* Allocator::allocateLogSizeClass(size_t size)
> 
> void* Allocator::allocateSlowCase(size_t size)
> {
>-    if (m_debugHeap)
>-        return m_debugHeap->malloc(size);
>-
>     if (size <= maskSizeClassMax) {
>         size_t sizeClass = bmalloc::maskSizeClass(size);
>         BumpAllocator& allocator = m_bumpAllocators[sizeClass];
>diff --git a/Source/bmalloc/bmalloc/Allocator.h b/Source/bmalloc/bmalloc/Allocator.h
>index 7c894de9b5c23c324155dc4ca7635c80af5c5d8d..933aae6ed8ea0d8a742eebdba4ce9f21cc53e782 100644
>--- a/Source/bmalloc/bmalloc/Allocator.h
>+++ b/Source/bmalloc/bmalloc/Allocator.h
>@@ -33,7 +33,6 @@
> namespace bmalloc {
> 
> class Deallocator;
>-class DebugHeap;
> class Heap;
> 
> // Per-cache object allocator.
>@@ -69,7 +68,6 @@ class Allocator {
>     std::array<BumpRangeCache, sizeClassCount> m_bumpRangeCaches;
> 
>     Heap& m_heap;
>-    DebugHeap* m_debugHeap;
>     Deallocator& m_deallocator;
> };
> 
>diff --git a/Source/bmalloc/bmalloc/Cache.cpp b/Source/bmalloc/bmalloc/Cache.cpp
>index c97b9095fb673b7f742471f681869cfc4c1a9353..29ad1309866f12d809f6f121d9f6511ed735362a 100644
>--- a/Source/bmalloc/bmalloc/Cache.cpp
>+++ b/Source/bmalloc/bmalloc/Cache.cpp
>@@ -25,11 +25,15 @@
> 
> #include "BInline.h"
> #include "Cache.h"
>+#include "DebugHeap.h"
>+#include "Environment.h"
> #include "Heap.h"
> #include "PerProcess.h"
> 
> namespace bmalloc {
> 
>+static DebugHeap* debugHeapCache { nullptr };
>+
> void Cache::scavenge(HeapKind heapKind)
> {
>     PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
>@@ -42,34 +46,82 @@ void Cache::scavenge(HeapKind heapKind)
>     caches->at(heapKind).deallocator().scavenge();
> }
> 
>+static BINLINE DebugHeap* debugHeap()
>+{
>+    if (debugHeapCache)
>+        return debugHeapCache;
>+    if (PerProcess<Environment>::get()->isDebugHeapEnabled()) {
>+        debugHeapCache = PerProcess<DebugHeap>::get();
>+        return debugHeapCache;
>+    }
>+    return nullptr;
>+}
>+
> Cache::Cache(HeapKind heapKind)
>     : m_deallocator(PerProcess<PerHeapKind<Heap>>::get()->at(heapKind))
>     , m_allocator(PerProcess<PerHeapKind<Heap>>::get()->at(heapKind), m_deallocator)
> {
>+    BASSERT(!PerProcess<Environment>::get()->isDebugHeapEnabled());
> }
> 
> BNO_INLINE void* Cache::tryAllocateSlowCaseNullCache(HeapKind heapKind, size_t size)
> {
>+    // FIXME: DebugHeap does not have tryAllocate feature.
>+    // https://bugs.webkit.org/show_bug.cgi?id=194837
>+    if (auto* heap = debugHeap())
>+        return heap->malloc(size);
>     return PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).allocator().tryAllocate(size);
> }
> 
> BNO_INLINE void* Cache::allocateSlowCaseNullCache(HeapKind heapKind, size_t size)
> {
>+    if (auto* heap = debugHeap())
>+        return heap->malloc(size);
>     return PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).allocator().allocate(size);
> }
> 
>+BNO_INLINE void* Cache::tryAllocateSlowCaseNullCache(HeapKind heapKind, size_t alignment, size_t size)
>+{
>+    if (auto* heap = debugHeap()) {
>+        constexpr bool crashOnFailure = false;
>+        return heap->memalign(alignment, size, crashOnFailure);
>+    }
>+    return PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).allocator().tryAllocate(alignment, size);
>+}
>+
> BNO_INLINE void* Cache::allocateSlowCaseNullCache(HeapKind heapKind, size_t alignment, size_t size)
> {
>+    if (auto* heap = debugHeap()) {
>+        constexpr bool crashOnFailure = true;
>+        return heap->memalign(alignment, size, crashOnFailure);
>+    }
>     return PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).allocator().allocate(alignment, size);
> }
> 
> BNO_INLINE void Cache::deallocateSlowCaseNullCache(HeapKind heapKind, void* object)
> {
>+    if (auto* heap = debugHeap()) {
>+        heap->free(object);
>+        return;
>+    }
>     PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).deallocator().deallocate(object);
> }
> 
>+BNO_INLINE void* Cache::tryReallocateSlowCaseNullCache(HeapKind heapKind, void* object, size_t newSize)
>+{
>+    if (auto* heap = debugHeap()) {
>+        constexpr bool crashOnFailure = false;
>+        return heap->realloc(object, newSize, crashOnFailure);
>+    }
>+    return PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).allocator().tryReallocate(object, newSize);
>+}
>+
> BNO_INLINE void* Cache::reallocateSlowCaseNullCache(HeapKind heapKind, void* object, size_t newSize)
> {
>+    if (auto* heap = debugHeap()) {
>+        constexpr bool crashOnFailure = true;
>+        return heap->realloc(object, newSize, crashOnFailure);
>+    }
>     return PerThread<PerHeapKind<Cache>>::getSlowCase()->at(mapToActiveHeapKind(heapKind)).allocator().reallocate(object, newSize);
> }
> 
>diff --git a/Source/bmalloc/bmalloc/Cache.h b/Source/bmalloc/bmalloc/Cache.h
>index fde7e09ed0395d8b701994fb898db8c0aa02c6a6..9f525a7fcf4ea9d806531e13ab0e55ce7682962e 100644
>--- a/Source/bmalloc/bmalloc/Cache.h
>+++ b/Source/bmalloc/bmalloc/Cache.h
>@@ -56,8 +56,10 @@ class Cache {
> private:
>     BEXPORT static void* tryAllocateSlowCaseNullCache(HeapKind, size_t);
>     BEXPORT static void* allocateSlowCaseNullCache(HeapKind, size_t);
>+    BEXPORT static void* tryAllocateSlowCaseNullCache(HeapKind, size_t alignment, size_t);
>     BEXPORT static void* allocateSlowCaseNullCache(HeapKind, size_t alignment, size_t);
>     BEXPORT static void deallocateSlowCaseNullCache(HeapKind, void*);
>+    BEXPORT static void* tryReallocateSlowCaseNullCache(HeapKind, void*, size_t);
>     BEXPORT static void* reallocateSlowCaseNullCache(HeapKind, void*, size_t);
> 
>     Deallocator m_deallocator;
>@@ -84,7 +86,7 @@ inline void* Cache::tryAllocate(HeapKind heapKind, size_t alignment, size_t size
> {
>     PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
>     if (!caches)
>-        return allocateSlowCaseNullCache(heapKind, alignment, size);
>+        return tryAllocateSlowCaseNullCache(heapKind, alignment, size);
>     return caches->at(mapToActiveHeapKindAfterEnsuringGigacage(heapKind)).allocator().tryAllocate(alignment, size);
> }
> 
>@@ -108,7 +110,7 @@ inline void* Cache::tryReallocate(HeapKind heapKind, void* object, size_t newSiz
> {
>     PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
>     if (!caches)
>-        return reallocateSlowCaseNullCache(heapKind, object, newSize);
>+        return tryReallocateSlowCaseNullCache(heapKind, object, newSize);
>     return caches->at(mapToActiveHeapKindAfterEnsuringGigacage(heapKind)).allocator().tryReallocate(object, newSize);
> }
> 
>diff --git a/Source/bmalloc/bmalloc/Deallocator.cpp b/Source/bmalloc/bmalloc/Deallocator.cpp
>index 556f3279d52a243ff0db2b34f55a3c0eaf92d303..58ce3a6a910f0530ec0d11ae8ee37ab79351303e 100644
>--- a/Source/bmalloc/bmalloc/Deallocator.cpp
>+++ b/Source/bmalloc/bmalloc/Deallocator.cpp
>@@ -27,7 +27,7 @@
> #include "BInline.h"
> #include "Chunk.h"
> #include "Deallocator.h"
>-#include "DebugHeap.h"
>+#include "Environment.h"
> #include "Heap.h"
> #include "Object.h"
> #include "PerProcess.h"
>@@ -39,13 +39,8 @@ namespace bmalloc {
> 
> Deallocator::Deallocator(Heap& heap)
>     : m_heap(heap)
>-    , m_debugHeap(heap.debugHeap())
> {
>-    if (m_debugHeap) {
>-        // Fill the object log in order to disable the fast path.
>-        while (m_objectLog.size() != m_objectLog.capacity())
>-            m_objectLog.push(nullptr);
>-    }
>+    BASSERT(!PerProcess<Environment>::get()->isDebugHeapEnabled());
> }
> 
> Deallocator::~Deallocator()
>@@ -55,9 +50,6 @@ Deallocator::~Deallocator()
>     
> void Deallocator::scavenge()
> {
>-    if (m_debugHeap)
>-        return;
>-
>     std::unique_lock<Mutex> lock(Heap::mutex());
> 
>     processObjectLog(lock);
>@@ -73,9 +65,6 @@ void Deallocator::processObjectLog(std::unique_lock<Mutex>& lock)
> 
> void Deallocator::deallocateSlowCase(void* object)
> {
>-    if (m_debugHeap)
>-        return m_debugHeap->free(object);
>-
>     if (!object)
>         return;
> 
>diff --git a/Source/bmalloc/bmalloc/Deallocator.h b/Source/bmalloc/bmalloc/Deallocator.h
>index 325d1df524ea28a43cc5f44f09c6d014149bc208..1342c4cafdc0f14552641ca23a14d53588056522 100644
>--- a/Source/bmalloc/bmalloc/Deallocator.h
>+++ b/Source/bmalloc/bmalloc/Deallocator.h
>@@ -33,7 +33,6 @@
> 
> namespace bmalloc {
> 
>-class DebugHeap;
> class Heap;
> class Mutex;
> 
>@@ -58,7 +57,6 @@ class Deallocator {
>     Heap& m_heap;
>     FixedVector<void*, deallocatorLogCapacity> m_objectLog;
>     LineCache m_lineCache; // The Heap removes items from this cache.
>-    DebugHeap* m_debugHeap;
> };
> 
> inline bool Deallocator::deallocateFastCase(void* object)

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194811: 362446 | 362447 | 362451 | 362459