Add retry logic to partition alloc

Source/wtf/PartitionAlloc.cpp

Index: Source/wtf/PartitionAlloc.cpp
diff --git a/Source/wtf/PartitionAlloc.cpp b/Source/wtf/PartitionAlloc.cpp
index 4a83b0efbb9b9ee3cab607f11140755856b73ad5..523d02bb488d91af4d55b824b504f4f8ca777a5a 100644
--- a/Source/wtf/PartitionAlloc.cpp
+++ b/Source/wtf/PartitionAlloc.cpp
@@ -31,6 +31,8 @@
 #include "config.h"
 #include "wtf/PartitionAlloc.h"
 
+#include "wtf/Atomics.h"
+
 #include <string.h>
 
 #ifndef NDEBUG
@@ -55,6 +57,11 @@ COMPILE_ASSERT(WTF::kGenericMaxBucketed == 983040, generic_max_bucketed);
 
 namespace WTF {
 
+enum PartitionAllocFlagsInternal {
+    // Flags after to start after all flags in PartitionAllocFlags.
+    PartitionAllocRetry = 1 << 1,
+};
+
 int PartitionRootBase::gInitializedLock = 0;
 bool PartitionRootBase::gInitialized = false;
 PartitionPage PartitionRootBase::gSeedPage;
@@ -301,6 +308,23 @@ static NEVER_INLINE void partitionOutOfMemory()
     IMMEDIATE_CRASH();
 }
 
+static PartitionFreeUpMemoryCallback s_partitionFreeUpMemoryCallback = nullptr;
+static volatile int s_partitionFreeUpMemoryRecusionCount = 0;
+
+void partitionSetFreeUpMemoryCallback(PartitionFreeUpMemoryCallback callback)
+{
+    s_partitionFreeUpMemoryCallback = callback;
+}
+
+static void partitionFreeUpMemory()
+{

[Chris Evans, 2014/11/06 21:45:34]

One more thing. Can we add here a // TODO comment that says we should also take
the opportunity to flush the ring list of unused but still committed pages?

+    if (atomicTestAndSetToOne(&s_partitionFreeUpMemoryRecusionCount))
+        return;
+    if (s_partitionFreeUpMemoryCallback)
+        s_partitionFreeUpMemoryCallback();

[Chris Evans, 2014/11/06 21:43:51]

Ah, so s_partitionFreeUpMemoryCallback() is going to by definition re-enter the
allocator, for example partitionFreeGeneric().

Currently, this won't work -- partitionFreeGeneric() will attempt to take the
lock, which will deadlock because it is already held. We're using a plain old
spinlock and not a recursive lock.

We could drop and then re-acquire the lock but that does make me a little
nervous. It may also be tricky, actually: at this level we don't know what type
of partition we're dealing with (generic, which has locks vs. sized which is
lockless).

I bet the code paths within the callback will be a little complicated. So I
would not be surprised if something tried to allocate memory. Aside from the
deadlock concern above, this would now raise the spectre of re-entering
partitionFreeUpMemory().

If we do proceed with this patch, we might want to define what our exact
re-entrancy expectations are and add some ASSERT()s to make sure the callback
implementation is not being naughty.

+    atomicSetOneToZero(&s_partitionFreeUpMemoryRecusionCount);
+}
+
 static ALWAYS_INLINE void partitionDecommitSystemPages(PartitionRootBase* root, void* addr, size_t len)
 {
     decommitSystemPages(addr, len);
@@ -694,6 +718,13 @@ void* partitionAllocSlowPath(PartitionRootBase* root, int flags, size_t size, Pa
     return partitionPageAllocAndFillFreelist(newPage);
 
 partitionAllocSlowPathFailed:
+    if (!(flags & PartitionAllocRetry)) {
+        partitionFreeUpMemory();

[Chris Evans, 2014/11/06 21:43:51]

Nit: I wonder if we want a comment here to say that this is tail re-entrancy and
to be careful not to rely on previously calculated state after this call?

+        void* ptr = partitionAllocSlowPath(root, flags | PartitionAllocRetry, size, bucket);

[Jens Widell, 2014/10/22 11:24:05]

Wouldn't it be better to retry partitionBucketAlloc() instead of
partitionAllocSlowPath()?

By trying partitionAllocSlowPath() you bypass the per-bucket freelist check, and
partitionAllocSlowPath() begins with an assert that that freelist is empty. If
memory has actually been freed, typically the freelist wouldn't be empty.

[Chris Evans, 2014/11/06 21:43:51]

I agree that partitionBucketAlloc() is the correct API to call.

+        if (ptr)
+            return ptr;
+    }
+
     if (returnNull)
         return nullptr;
     partitionOutOfMemory();