Changeset 133119 in webkit

Timestamp:

Oct 31, 2012, 8:21:02 PM (13 years ago)

Author:

cevans@google.com

Message:

RenderArena has a memory leak and poor efficiency
​https://bugs.webkit.org/show_bug.cgi?id=100893

Reviewed by Eric Seidel.

1) Avoid memory leak that persists for the Document lifetime by
increasing recycled size buckets up to 1024. It was previously 400,
and sizeof(RenderNamedFlowThread) / sizeof(RenderSVGText) both blew this
quota. An assert was added to prevent this happening again.

2) Fix the size of the recyled size bucket array on 64-bit. We only
need 8 byte granularity on 64-bit, but we had 4.

3) Try and pass power-of-two sizes to the underlying malloc() call, so
that we're space efficient. We now take Arena metadata into account.

4) Double the default RenderArena size allocation to 8192 bytes. Even
for a render of a trivial text file, 4096 bytes is not enough to prevent
extra calls into the underlying malloc() for more arena pool.

• rendering/RenderArena.cpp:

(WebCore::RenderArena::RenderArena): Adjust arena size so that we pass on the page-sized multiple to the underlying malloc() implementation.
(WebCore::RenderArena::allocate):
(WebCore::RenderArena::free): Assert that the allocation size is handled by our recycling buckets.

• rendering/RenderArena.h:

(WebCore): Maintain free buckets up to 1024 bytes to avoid memory leak.
(RenderArena): Double the default allocation size and handle 64-bit systems more efficiently.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderArena.cpp (modified) (5 diffs)
• rendering/RenderArena.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-31  Chris Evans  <cevans@google.com>
+
+        RenderArena has a memory leak and poor efficiency
+        https://bugs.webkit.org/show_bug.cgi?id=100893
+
+        Reviewed by Eric Seidel.
+
+        1) Avoid memory leak that persists for the Document lifetime by
+        increasing recycled size buckets up to 1024. It was previously 400,
+        and sizeof(RenderNamedFlowThread) / sizeof(RenderSVGText) both blew this
+        quota. An assert was added to prevent this happening again.
+
+        2) Fix the size of the recyled size bucket array on 64-bit. We only
+        need 8 byte granularity on 64-bit, but we had 4.
+
+        3) Try and pass power-of-two sizes to the underlying malloc() call, so
+        that we're space efficient. We now take Arena metadata into account.
+
+        4) Double the default RenderArena size allocation to 8192 bytes. Even
+        for a render of a trivial text file, 4096 bytes is not enough to prevent
+        extra calls into the underlying malloc() for more arena pool.
+
+        * rendering/RenderArena.cpp:
+        (WebCore::RenderArena::RenderArena): Adjust arena size so that we pass on the page-sized multiple to the underlying malloc() implementation.
+        (WebCore::RenderArena::allocate):
+        (WebCore::RenderArena::free): Assert that the allocation size is handled by our recycling buckets.
+        * rendering/RenderArena.h:
+        (WebCore): Maintain free buckets up to 1024 bytes to avoid memory leak.
+        (RenderArena): Double the default allocation size and handle 64-bit systems more efficiently.
+
 2012-10-31  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/rendering/RenderArena.cpp

@@ -71 +71 @@
     , m_totalAllocated(0)
 {
+    ASSERT(arenaSize > sizeof(Arena) + ARENA_ALIGN_MASK);
+    // The underlying Arena class allocates some metadata on top of our
+    // requested size. Factor this in so that we can get perfect power-of-two
+    // allocation sizes passed to the underlying malloc() call.
+    arenaSize -= (sizeof(Arena) + ARENA_ALIGN_MASK);
     // Initialize the arena pool
     INIT_ARENA_POOL(&m_pool, "RenderArena", arenaSize);
@@ -98 +103 @@
 void* RenderArena::allocate(size_t size)
 {
+    ASSERT(size <= gMaxRecycledSize - 32);
     m_totalSize += size;
 
@@ -112 +118 @@
     return static_cast<char*>(block) + debugHeaderSize;
 #else
-    void* result = 0;
-
     // Ensure we have correct alignment for pointers.  Important for Tru64
     size = ROUNDUP(size, sizeof(void*));
 
-    // Check recyclers first
-    if (size < gMaxRecycledSize) {
-        const size_t index = size >> 2;
+    const size_t index = size >> kRecyclerShift;
 
-        result = m_recyclers[index];
-        if (result) {
-            // Need to move to the next object
-            void* next = MaskPtr(*((void**)result), m_mask);
-            m_recyclers[index] = next;
-        }
+    void* result = m_recyclers[index];
+    if (result) {
+        // Need to move to the next object
+        void* next = MaskPtr(*((void**)result), m_mask);
+        m_recyclers[index] = next;
     }
 
@@ -142 +143 @@
 void RenderArena::free(size_t size, void* ptr)
 {
+    ASSERT(size <= gMaxRecycledSize - 32);
     m_totalSize -= size;
 
@@ -159 +161 @@
     size = ROUNDUP(size, sizeof(void*));
 
-    // See if it's a size that we recycle
-    if (size < gMaxRecycledSize) {
-        const size_t index = size >> 2;
-        void* currentTop = m_recyclers[index];
-        m_recyclers[index] = ptr;
-        *((void**)ptr) = MaskPtr(currentTop, m_mask);
-    }
+    const size_t index = size >> kRecyclerShift;
+    void* currentTop = m_recyclers[index];
+    m_recyclers[index] = ptr;
+    *((void**)ptr) = MaskPtr(currentTop, m_mask);
 #endif
 }

trunk/Source/WebCore/rendering/RenderArena.h

@@ -42 +42 @@
 namespace WebCore {
 
-static const size_t gMaxRecycledSize = 400;
+static const size_t gMaxRecycledSize = 1024;
 
 class RenderArena {
     WTF_MAKE_NONCOPYABLE(RenderArena); WTF_MAKE_FAST_ALLOCATED;
 public:
-    RenderArena(unsigned arenaSize = 4096);
+    RenderArena(unsigned arenaSize = 8192);
     ~RenderArena();
 
@@ -63 +63 @@
     // The mask used to secure the recycled freelist pointers.
     uintptr_t m_mask;
-    // The recycler array is sparse with the indices being multiples of 4,
-    // i.e., 0, 4, 8, 12, 16, 20, ...
-    void* m_recyclers[gMaxRecycledSize >> 2];
+    // The recycler array is sparse with the indices being multiples of the
+    // rounding size, sizeof(void*), i.e., 0, 4, 8, 12, 16, 20, ... on 32-bit.
+    static const size_t kRecyclerShift = (sizeof(void*) == 8) ? 3 : 2;
+    void* m_recyclers[gMaxRecycledSize >> kRecyclerShift];
 
     size_t m_totalSize;