- Revision
- 228107
- Author
- commit-qu...@webkit.org
- Date
- 2018-02-05 11:18:45 -0800 (Mon, 05 Feb 2018)
Log Message
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: trunk/Source/bmalloc/ChangeLog (228106 => 228107)
--- trunk/Source/bmalloc/ChangeLog 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/ChangeLog 2018-02-05 19:18:45 UTC (rev 228107)
@@ -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: trunk/Source/bmalloc/bmalloc/Heap.cpp (228106 => 228107)
--- trunk/Source/bmalloc/bmalloc/Heap.cpp 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/bmalloc/Heap.cpp 2018-02-05 19:18:45 UTC (rev 228107)
@@ -64,7 +64,7 @@
#endif
}
- m_scavenger = PerProcess<Scavenger>::get();
+ m_scavenger = SafePerProcess<Scavenger>::get();
}
bool Heap::usingGigacage()
Modified: trunk/Source/bmalloc/bmalloc/IsoDirectoryInlines.h (228106 => 228107)
--- trunk/Source/bmalloc/bmalloc/IsoDirectoryInlines.h 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/bmalloc/IsoDirectoryInlines.h 2018-02-05 19:18:45 UTC (rev 228107)
@@ -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: trunk/Source/bmalloc/bmalloc/PerProcess.h (228106 => 228107)
--- trunk/Source/bmalloc/bmalloc/PerProcess.h 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/bmalloc/PerProcess.h 2018-02-05 19:18:45 UTC (rev 228107)
@@ -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: trunk/Source/bmalloc/bmalloc/Scavenger.cpp (228106 => 228107)
--- trunk/Source/bmalloc/bmalloc/Scavenger.cpp 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/bmalloc/Scavenger.cpp 2018-02-05 19:18:45 UTC (rev 228107)
@@ -32,6 +32,8 @@
namespace bmalloc {
+DEFINE_SAFE_PER_PROCESS_STORAGE(Scavenger);
+
Scavenger::Scavenger(std::lock_guard<StaticMutex>&)
{
#if BOS(DARWIN)
Modified: trunk/Source/bmalloc/bmalloc/Scavenger.h (228106 => 228107)
--- trunk/Source/bmalloc/bmalloc/Scavenger.h 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/bmalloc/Scavenger.h 2018-02-05 19:18:45 UTC (rev 228107)
@@ -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: trunk/Source/bmalloc/bmalloc/bmalloc.cpp (228106 => 228107)
--- trunk/Source/bmalloc/bmalloc/bmalloc.cpp 2018-02-05 19:15:10 UTC (rev 228106)
+++ trunk/Source/bmalloc/bmalloc/bmalloc.cpp 2018-02-05 19:18:45 UTC (rev 228107)
@@ -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
