PartitionAlloc: Distinguish OOMs where a lot of super pages are not committed

Source/wtf/PartitionAlloc.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 278 matching lines @@
     size_t i;
     for (i = 0; i < kGenericNumBucketedOrders * kGenericNumBucketsPerOrder; ++i) {
         PartitionBucket* bucket = &root->buckets[i];
         if (!partitionAllocShutdownBucket(bucket))
             noLeaks = false;
     }
     partitionAllocBaseShutdown(root);
     return noLeaks;
 }
 
-static NEVER_INLINE void partitionOutOfMemory()
+static NEVER_INLINE void partitionOutOfMemory(const PartitionRootBase* root)
 {
+    if ((uint64_t)root->totalSizeOfSuperPages - (uint64_t)root->totalSizeOfCommittedPages > kReasonableSizeOfUnusedPages) {

[Tom Sepez, 2014/11/06 17:32:51]

why this promotion to unit64_t? Also, are we sure that
root->totalSizeOfSuperPages >= root->totalSizeOfCommittedPages

[Chris Evans, 2014/11/06 20:43:22]

I don't think this condition will ever give more accurate results on 64-bit. I
propose #if CPU(32_BIT) for this entire if condition.

[Chris Evans, 2014/11/06 20:43:22]

I also don't like the look of these casts. Hopefully we can get rid of them. If
not, the coding style is wrong: we always use static_cast.

I do believe root->totalSizeOfSuperPages >= root->totalSizeOfCommittedPages
should be an invariant that holds. Maybe add some ASSERT() at the code locations
where we increase root->totalSizeOfCommittedPages to ASSERT it is still <=
root->totalSizeOfSuperPages ?

[Jens Widell, 2014/11/07 08:31:21]

Since we now include direct mapped allocations in totalSizeOfCommittedPages, it
can theoretically be bigger than totalSizeOfSuperPages. In theory, we could have
*only* direct mapped allocations, in which case totalSizeOfSuperPages==0 and
totalSizeOfCommittedPages>0.

[hiroshige, 2014/11/07 08:57:00]

I expect there are no out-of-virtual-address-space crash detected here, but I'd
like to enable this check in 64-bit to confirm that.

[hiroshige, 2014/11/07 08:57:00]

Added assertions, and removed casts.

[hiroshige, 2014/11/07 08:57:00]

Yes, and I added totalSizeOfDirectMappedPages in Patch Set 5.

+        // OOMs where a lot of super pages are allocated but not committed,
+        // probably due to http://crbug.com/421387.
+        // Crash at a special address (0x9b)
+        // to be easily distinguished on crash reports.
+        ((void(*)())0x0000009b)();

[Tom Sepez, 2014/11/06 17:32:51]

Maybe we need IMMEDIATE_CRASH_WITH_FLAG(0x9b) macro along the lines of
immediate_crash since there may be platforms where we want to use a builtin.

[Chris Evans, 2014/11/06 20:43:22]

I don't like this, it's a bit arbitrary and doesn't lead to an obviously visible
distinction in crash logs.

I'd prefer we did it this way:

partitionOutOfMemoryWithLotsOfUncommitedPages();

With:

static NEVER_INLINE void partitionOutOfMemoryWithLotsOfUncommitedPages()
{
    IMMEDIATE_CRASH();
}

[hiroshige, 2014/11/07 08:56:59]

cevans@, Agree, I also prefer what you suggested, but in Windows  many functions
(partitionOutOfMemoryWithLotsOfUncommitedPages in this case) won't appear in the
crash stack traces, even with the NEVER_INLINE directive.
I'm not sure why (frame pointer omission?), but in the crash reports, there are
no partitionOutOfMemory (or partitionFull) reported on Windows.

Does anyone knows how to work around this?

[hiroshige, 2014/11/07 10:28:17]

Added IMMEDIATE_CRASH_WITH_FLAG and
partitionOutOfMemoryWithLotsOfUncommitedPages() in Patch Set 6.

+    }
+    // Ordinary OOMs (where super pages are consumed and mostly committed).
     IMMEDIATE_CRASH();
 }
 
 static ALWAYS_INLINE void partitionDecommitSystemPages(PartitionRootBase* root, void* addr, size_t len)
 {
     decommitSystemPages(addr, len);
     ASSERT(root->totalSizeOfCommittedPages > len);
     root->totalSizeOfCommittedPages -= len;
 }
 
@@ skipping 383 matching lines @@
         newPage = partitionPointerToPageNoAlignmentCheck(rawNewPage);
     }
 
     partitionPageReset(newPage, bucket);
     bucket->activePagesHead = newPage;
     return partitionPageAllocAndFillFreelist(newPage);
 
 partitionAllocSlowPathFailed:
     if (returnNull)
         return nullptr;
-    partitionOutOfMemory();
+    partitionOutOfMemory(root);
     return nullptr;
 }
 
 static ALWAYS_INLINE void partitionFreePage(PartitionRootBase* root, PartitionPage* page)
 {
     ASSERT(page->freelistHead);
     ASSERT(!page->numAllocatedSlots);
     partitionUnusePage(root, page);
     // We actually leave the freed page in the active list. We'll sweep it on
     // to the free page list when we next walk the active page list. Pulling
@@ skipping 258 matching lines @@
     printf("total live: %zu bytes\n", totalLive);
     printf("total resident: %zu bytes\n", totalResident);
     printf("total freeable: %zu bytes\n", totalFreeable);
     fflush(stdout);
 }
 
 #endif // !NDEBUG
 
 } // namespace WTF