[bmalloc] bmalloc::Cache should not be instantiated if we are using system malloc
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Feb 2019 21:30:40 +0000 (21:30 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Feb 2019 21:30:40 +0000 (21:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194811

Reviewed by Mark Lam.

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.

* bmalloc/bmalloc.cpp:
(bmalloc::api::isEnabled):
We used `getFastCase()` for Heap. But it is basically wrong since we do not have any guarantee that someone already initializes
Heap when this is called. Previously, luckily, Cache is initialized, and Cache initialized Heap. But Cache initialization is removed
for system malloc mode and now PerProcess<PerHeapKind<Heap>>::getFastCase() returns nullptr at an early phase. This patch just uses
Environment::isDebugHeapEnabled() instead.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241832 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/bmalloc/ChangeLog
Source/bmalloc/bmalloc/Allocator.cpp
Source/bmalloc/bmalloc/Allocator.h
Source/bmalloc/bmalloc/Cache.cpp
Source/bmalloc/bmalloc/Cache.h
Source/bmalloc/bmalloc/Deallocator.cpp
Source/bmalloc/bmalloc/Deallocator.h
Source/bmalloc/bmalloc/bmalloc.cpp

index b0bfd65..98efb21 100644 (file)
@@ -1,3 +1,53 @@
+2019-02-20  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 Mark Lam.
+
+        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.
+
+        * bmalloc/bmalloc.cpp:
+        (bmalloc::api::isEnabled):
+        We used `getFastCase()` for Heap. But it is basically wrong since we do not have any guarantee that someone already initializes
+        Heap when this is called. Previously, luckily, Cache is initialized, and Cache initialized Heap. But Cache initialization is removed
+        for system malloc mode and now PerProcess<PerHeapKind<Heap>>::getFastCase() returns nullptr at an early phase. This patch just uses
+        Environment::isDebugHeapEnabled() instead.
+
 2019-02-20  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r241789.
index 4fbdb09..cfd01fe 100644 (file)
@@ -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];
index 7c894de..933aae6 100644 (file)
@@ -33,7 +33,6 @@
 namespace bmalloc {
 
 class Deallocator;
-class DebugHeap;
 class Heap;
 
 // Per-cache object allocator.
@@ -69,7 +68,6 @@ private:
     std::array<BumpRangeCache, sizeClassCount> m_bumpRangeCaches;
 
     Heap& m_heap;
-    DebugHeap* m_debugHeap;
     Deallocator& m_deallocator;
 };
 
index c97b909..29ad130 100644 (file)
 
 #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);
 }
 
index fde7e09..9f525a7 100644 (file)
@@ -56,8 +56,10 @@ public:
 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);
 }
 
index 556f327..58ce3a6 100644 (file)
@@ -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;
 
index 325d1df..1342c4c 100644 (file)
@@ -33,7 +33,6 @@
 
 namespace bmalloc {
 
-class DebugHeap;
 class Heap;
 class Mutex;
 
@@ -58,7 +57,6 @@ private:
     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)
index 620dbfc..0663ef3 100644 (file)
@@ -25,6 +25,7 @@
 
 #include "bmalloc.h"
 
+#include "Environment.h"
 #include "PerProcess.h"
 
 namespace bmalloc { namespace api {
@@ -87,11 +88,9 @@ void scavenge()
     PerProcess<Scavenger>::get()->scavenge();
 }
 
-bool isEnabled(HeapKind kind)
+bool isEnabled(HeapKind)
 {
-    kind = mapToActiveHeapKind(kind);
-    std::unique_lock<Mutex> lock(Heap::mutex());
-    return !PerProcess<PerHeapKind<Heap>>::getFastCase()->at(kind).debugHeap();
+    return !PerProcess<Environment>::get()->isDebugHeapEnabled();
 }
 
 #if BOS(DARWIN)