Title: [228619] releases/WebKitGTK/webkit-2.20/Source/bmalloc
Revision
228619
Author
carlo...@webkit.org
Date
2018-02-19 01:12:03 -0800 (Mon, 19 Feb 2018)

Log Message

Merge r228107 - Multiple bmalloc scavenger threads is unexpected
https://bugs.webkit.org/show_bug.cgi?id=182474
<rdar://problem/37175526>

Patch by Joseph Pecoraro <pecor...@apple.com> on 2018-02-05
Reviewed by Filip Pizlo.

* bmalloc/Heap.cpp:
(bmalloc::Heap::Heap):
* bmalloc/IsoDirectoryInlines.h:
(bmalloc::passedNumPages>::takeFirstEligible):
(bmalloc::passedNumPages>::didBecome):
* bmalloc/bmalloc.cpp:
(bmalloc::api::scavenge):
(bmalloc::api::setScavengerThreadQOSClass):
Switch to SafePerProcess for Scavenger to ensure one instance
for the entire process.

* bmalloc/PerProcess.h:
(bmalloc::PerProcess::get):
(bmalloc::PerProcess::getFastCase):
(bmalloc::PerProcess::getSlowCase):
(bmalloc::SafePerProcess::get):
(bmalloc::SafePerProcess::getFastCase):
(bmalloc::SafePerProcess::getSlowCase):
Duplicate the class with a version that can ensure
single instances by requiring exporting symbols that
can be created with macros.

* bmalloc/Scavenger.cpp:
* bmalloc/Scavenger.h:
Export symbols to ensure all images get the same instance.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/ChangeLog (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/ChangeLog	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/ChangeLog	2018-02-19 09:12:03 UTC (rev 228619)
@@ -1,3 +1,37 @@
+2018-02-05  Joseph Pecoraro  <pecor...@apple.com>
+
+        Multiple bmalloc scavenger threads is unexpected
+        https://bugs.webkit.org/show_bug.cgi?id=182474
+        <rdar://problem/37175526>
+
+        Reviewed by Filip Pizlo.
+
+        * bmalloc/Heap.cpp:
+        (bmalloc::Heap::Heap):
+        * bmalloc/IsoDirectoryInlines.h:
+        (bmalloc::passedNumPages>::takeFirstEligible):
+        (bmalloc::passedNumPages>::didBecome):
+        * bmalloc/bmalloc.cpp:
+        (bmalloc::api::scavenge):
+        (bmalloc::api::setScavengerThreadQOSClass):
+        Switch to SafePerProcess for Scavenger to ensure one instance
+        for the entire process.
+
+        * bmalloc/PerProcess.h:
+        (bmalloc::PerProcess::get):
+        (bmalloc::PerProcess::getFastCase):
+        (bmalloc::PerProcess::getSlowCase):
+        (bmalloc::SafePerProcess::get):
+        (bmalloc::SafePerProcess::getFastCase):
+        (bmalloc::SafePerProcess::getSlowCase):
+        Duplicate the class with a version that can ensure
+        single instances by requiring exporting symbols that
+        can be created with macros.
+
+        * bmalloc/Scavenger.cpp:
+        * bmalloc/Scavenger.h:
+        Export symbols to ensure all images get the same instance.
+
 2018-01-31  Saam Barati  <sbar...@apple.com>
 
         Replace tryLargeMemalignVirtual with tryLargeZeroedMemalignVirtual and use it to allocate large zeroed memory in Wasm

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Heap.cpp (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Heap.cpp	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Heap.cpp	2018-02-19 09:12:03 UTC (rev 228619)
@@ -64,7 +64,7 @@
 #endif
     }
     
-    m_scavenger = PerProcess<Scavenger>::get();
+    m_scavenger = SafePerProcess<Scavenger>::get();
 }
 
 bool Heap::usingGigacage()

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/IsoDirectoryInlines.h (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/IsoDirectoryInlines.h	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/IsoDirectoryInlines.h	2018-02-19 09:12:03 UTC (rev 228619)
@@ -51,7 +51,7 @@
     if (pageIndex >= numPages)
         return EligibilityKind::Full;
     
-    Scavenger& scavenger = *PerProcess<Scavenger>::get();
+    Scavenger& scavenger = *SafePerProcess<Scavenger>::get();
     scavenger.didStartGrowing();
     
     IsoPage<Config>* page = m_pages[pageIndex];
@@ -100,7 +100,7 @@
         if (verbose)
             fprintf(stderr, "%p: %p did become empty.\n", this, page);
         m_empty[pageIndex] = true;
-        PerProcess<Scavenger>::get()->schedule(IsoPageBase::pageSize);
+        SafePerProcess<Scavenger>::get()->schedule(IsoPageBase::pageSize);
         return;
     }
     BCRASH();

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/PerProcess.h (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/PerProcess.h	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/PerProcess.h	2018-02-19 09:12:03 UTC (rev 228619)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -23,8 +23,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
  */
 
-#ifndef PerProcess_h
-#define PerProcess_h
+#pragma once
 
 #include "BInline.h"
 #include "Sizes.h"
@@ -39,6 +38,10 @@
 //
 // Object will be instantiated only once, even in the face of concurrency.
 //
+// WARNING: PerProcess<T> does not export its storage. So in actuality when
+// used in multiple libraries / images it ends up being per-image. To truly
+// declare a per-process singleton use SafePerProcess<T>.
+//
 // NOTE: If you observe global side-effects of the Object constructor, be
 // sure to lock the Object mutex. For example:
 //
@@ -55,13 +58,31 @@
 template<typename T>
 class PerProcess {
 public:
-    static T* get();
-    static T* getFastCase();
+    static T* get()
+    {
+        T* object = getFastCase();
+        if (!object)
+            return getSlowCase();
+        return object;
+    }
+
+    static T* getFastCase()
+    {
+        return s_object.load(std::memory_order_relaxed);
+    }
     
     static StaticMutex& mutex() { return s_mutex; }
 
 private:
-    static T* getSlowCase();
+    BNO_INLINE static T* getSlowCase()
+    {
+        std::lock_guard<StaticMutex> lock(s_mutex);
+        if (!s_object.load(std::memory_order_consume)) {
+            T* t = new (&s_memory) T(lock);
+            s_object.store(t, std::memory_order_release);
+        }
+        return s_object.load(std::memory_order_consume);
+    }
 
     static std::atomic<T*> s_object;
     static StaticMutex s_mutex;
@@ -71,40 +92,67 @@
 };
 
 template<typename T>
-BINLINE T* PerProcess<T>::getFastCase()
-{
-    return s_object.load(std::memory_order_relaxed);
-}
+std::atomic<T*> PerProcess<T>::s_object;
 
 template<typename T>
-BINLINE T* PerProcess<T>::get()
-{
-    T* object = getFastCase();
-    if (!object)
-        return getSlowCase();
-    return object;
-}
+StaticMutex PerProcess<T>::s_mutex;
 
 template<typename T>
-BNO_INLINE T* PerProcess<T>::getSlowCase()
-{
-    std::lock_guard<StaticMutex> lock(s_mutex);
-    if (!s_object.load(std::memory_order_consume)) {
-        T* t = new (&s_memory) T(lock);
-        s_object.store(t, std::memory_order_release);
-    }
-    return s_object.load(std::memory_order_consume);
-}
+typename PerProcess<T>::Memory PerProcess<T>::s_memory;
 
-template<typename T>
-std::atomic<T*> PerProcess<T>::s_object;
 
-template<typename T>
-StaticMutex PerProcess<T>::s_mutex;
+// SafePerProcess<T> behaves like PerProcess<T>, but its usage
+// requires DECLARE/DEFINE macros that export symbols that allow for
+// a single shared instance is across images in the process.
 
+template<typename T> struct SafePerProcessStorageTraits;
+
 template<typename T>
-typename PerProcess<T>::Memory PerProcess<T>::s_memory;
+class BEXPORT SafePerProcess {
+public:
+    using Storage = typename SafePerProcessStorageTraits<T>::Storage;
 
+    static T* get()
+    {
+        T* object = getFastCase();
+        if (!object)
+            return getSlowCase();
+        return object;
+    }
+
+    static T* getFastCase()
+    {
+        return (Storage::s_object).load(std::memory_order_relaxed);
+    }
+
+    static StaticMutex& mutex() { return Storage::s_mutex; }
+
+    using Memory = typename std::aligned_storage<sizeof(T), std::alignment_of<T>::value>::type;
+
+private:
+    BNO_INLINE static T* getSlowCase()
+    {
+        std::lock_guard<StaticMutex> lock(Storage::s_mutex);
+        if (!Storage::s_object.load(std::memory_order_consume)) {
+            T* t = new (&Storage::s_memory) T(lock);
+            Storage::s_object.store(t, std::memory_order_release);
+        }
+        return Storage::s_object.load(std::memory_order_consume);
+    }
+};
+
+#define DECLARE_SAFE_PER_PROCESS_STORAGE(Type) \
+    template<> struct SafePerProcessStorageTraits<Type> { \
+        struct BEXPORT Storage { \
+            BEXPORT static std::atomic<Type*> s_object; \
+            BEXPORT static StaticMutex s_mutex; \
+            BEXPORT static SafePerProcess<Type>::Memory s_memory; \
+        }; \
+    };
+
+#define DEFINE_SAFE_PER_PROCESS_STORAGE(Type) \
+    std::atomic<Type*> SafePerProcessStorageTraits<Type>::Storage::s_object; \
+    StaticMutex SafePerProcessStorageTraits<Type>::Storage::s_mutex; \
+    SafePerProcess<Type>::Memory SafePerProcessStorageTraits<Type>::Storage::s_memory;
+
 } // namespace bmalloc
-
-#endif // PerProcess_h

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Scavenger.cpp (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Scavenger.cpp	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Scavenger.cpp	2018-02-19 09:12:03 UTC (rev 228619)
@@ -32,6 +32,8 @@
 
 namespace bmalloc {
 
+DEFINE_SAFE_PER_PROCESS_STORAGE(Scavenger);
+
 Scavenger::Scavenger(std::lock_guard<StaticMutex>&)
 {
 #if BOS(DARWIN)

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Scavenger.h (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Scavenger.h	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/Scavenger.h	2018-02-19 09:12:03 UTC (rev 228619)
@@ -28,6 +28,7 @@
 #include "BPlatform.h"
 #include "DeferredDecommit.h"
 #include "Mutex.h"
+#include "PerProcess.h"
 #include "Vector.h"
 #include <condition_variable>
 #include <mutex>
@@ -92,6 +93,8 @@
     Vector<DeferredDecommit> m_deferredDecommits;
 };
 
+DECLARE_SAFE_PER_PROCESS_STORAGE(Scavenger);
+
 } // namespace bmalloc
 
 

Modified: releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/bmalloc.cpp (228618 => 228619)


--- releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/bmalloc.cpp	2018-02-19 09:08:12 UTC (rev 228618)
+++ releases/WebKitGTK/webkit-2.20/Source/bmalloc/bmalloc/bmalloc.cpp	2018-02-19 09:12:03 UTC (rev 228619)
@@ -73,7 +73,7 @@
 {
     scavengeThisThread();
 
-    PerProcess<Scavenger>::get()->scavenge();
+    SafePerProcess<Scavenger>::get()->scavenge();
 }
 
 bool isEnabled(HeapKind kind)
@@ -87,7 +87,7 @@
 void setScavengerThreadQOSClass(qos_class_t overrideClass)
 {
     std::unique_lock<StaticMutex> lock(Heap::mutex());
-    PerProcess<Scavenger>::get()->setScavengerThreadQOSClass(overrideClass);
+    SafePerProcess<Scavenger>::get()->setScavengerThreadQOSClass(overrideClass);
 }
 #endif
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to