Title: [241832] trunk/Source/bmalloc
Revision
241832
Author
ysuz...@apple.com
Date
2019-02-20 13:30:40 -0800 (Wed, 20 Feb 2019)

Log Message

[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.

Modified Paths

Diff

Modified: trunk/Source/bmalloc/ChangeLog (241831 => 241832)


--- trunk/Source/bmalloc/ChangeLog	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/ChangeLog	2019-02-20 21:30:40 UTC (rev 241832)
@@ -1,3 +1,53 @@
+2019-02-20  Yusuke Suzuki  <ysuz...@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-qu...@webkit.org>
 
         Unreviewed, rolling out r241789.

Modified: trunk/Source/bmalloc/bmalloc/Allocator.cpp (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/Allocator.cpp	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/Allocator.cpp	2019-02-20 21:30:40 UTC (rev 241832)
@@ -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 @@
 
 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 @@
 
 void* Allocator::tryAllocate(size_t size)
 {
-    if (m_debugHeap)
-        return m_debugHeap->malloc(size);
-
     if (size <= smallMax)
         return allocate(size);
 
@@ -78,9 +75,6 @@
 {
     BASSERT(isPowerOfTwo(alignment));
 
-    if (m_debugHeap)
-        return m_debugHeap->memalign(alignment, size, crashOnFailure);
-
     if (!size)
         size = alignment;
 
@@ -107,9 +101,6 @@
 
 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 @@
 
 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];

Modified: trunk/Source/bmalloc/bmalloc/Allocator.h (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/Allocator.h	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/Allocator.h	2019-02-20 21:30:40 UTC (rev 241832)
@@ -33,7 +33,6 @@
 namespace bmalloc {
 
 class Deallocator;
-class DebugHeap;
 class Heap;
 
 // Per-cache object allocator.
@@ -69,7 +68,6 @@
     std::array<BumpRangeCache, sizeClassCount> m_bumpRangeCaches;
 
     Heap& m_heap;
-    DebugHeap* m_debugHeap;
     Deallocator& m_deallocator;
 };
 

Modified: trunk/Source/bmalloc/bmalloc/Cache.cpp (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/Cache.cpp	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/Cache.cpp	2019-02-20 21:30:40 UTC (rev 241832)
@@ -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 @@
     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);
 }
 

Modified: trunk/Source/bmalloc/bmalloc/Cache.h (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/Cache.h	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/Cache.h	2019-02-20 21:30:40 UTC (rev 241832)
@@ -56,8 +56,10 @@
 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 @@
 {
     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 @@
 {
     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);
 }
 

Modified: trunk/Source/bmalloc/bmalloc/Deallocator.cpp (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/Deallocator.cpp	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/Deallocator.cpp	2019-02-20 21:30:40 UTC (rev 241832)
@@ -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 @@
 
 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 @@
     
 void Deallocator::scavenge()
 {
-    if (m_debugHeap)
-        return;
-
     std::unique_lock<Mutex> lock(Heap::mutex());
 
     processObjectLog(lock);
@@ -73,9 +65,6 @@
 
 void Deallocator::deallocateSlowCase(void* object)
 {
-    if (m_debugHeap)
-        return m_debugHeap->free(object);
-
     if (!object)
         return;
 

Modified: trunk/Source/bmalloc/bmalloc/Deallocator.h (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/Deallocator.h	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/Deallocator.h	2019-02-20 21:30:40 UTC (rev 241832)
@@ -33,7 +33,6 @@
 
 namespace bmalloc {
 
-class DebugHeap;
 class Heap;
 class Mutex;
 
@@ -58,7 +57,6 @@
     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)

Modified: trunk/Source/bmalloc/bmalloc/bmalloc.cpp (241831 => 241832)


--- trunk/Source/bmalloc/bmalloc/bmalloc.cpp	2019-02-20 21:24:56 UTC (rev 241831)
+++ trunk/Source/bmalloc/bmalloc/bmalloc.cpp	2019-02-20 21:30:40 UTC (rev 241832)
@@ -25,6 +25,7 @@
 
 #include "bmalloc.h"
 
+#include "Environment.h"
 #include "PerProcess.h"
 
 namespace bmalloc { namespace api {
@@ -87,11 +88,9 @@
     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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to