Bug 675150 - Avoid wasted space in JSArenaPools due to jemalloc rounding up, take 2. r=cdleary.
☠☠ backed out by e76839f98b18 ☠ ☠
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 07 Sep 2011 16:49:28 -0700
changeset 76699 240cfe9e5c2c2438bad144cb499359e60161ebfd
parent 76698 6e321c2f2a075f97f5f84ef4d9e3bbf96bcd20f5
child 76700 aa0168e8e389d4860ce9e83fc63dc8cc6d32fe84
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewerscdleary
bugs675150
milestone9.0a1
Bug 675150 - Avoid wasted space in JSArenaPools due to jemalloc rounding up, take 2. r=cdleary.
js/src/jsarena.cpp
js/src/jsarena.h
js/src/jscntxt.cpp
js/src/jscompartment.cpp
--- a/js/src/jsarena.cpp
+++ b/js/src/jsarena.cpp
@@ -54,30 +54,33 @@
 using namespace js;
 
 /* If JSArena's length is a multiple of 8, that ensures its payload is 8-aligned. */
 JS_STATIC_ASSERT(sizeof(JSArena) % 8 == 0);
 
 JS_PUBLIC_API(void)
 JS_InitArenaPool(JSArenaPool *pool, const char *name, size_t size, size_t align)
 {
+    JS_ASSERT(JS_CeilingLog2(size) == JS_FloorLog2(size));
     /* Restricting ourselves to some simple alignments keeps things simple. */
     if (align == 1 || align == 2 || align == 4 || align == 8) {
         pool->mask = align - 1;
     } else {
         /* This shouldn't happen, but set pool->mask reasonably if it does. */
         JS_NOT_REACHED("JS_InitArenaPool: bad align");
         pool->mask = 7;
     }
     pool->first.next = NULL;
     /* pool->first is a zero-sized dummy arena that's never allocated from. */
     pool->first.base = pool->first.avail = pool->first.limit =
         JS_ARENA_ALIGN(pool, &pool->first + 1);
     pool->current = &pool->first;
-    pool->arenasize = size;
+    pool->netsize = size - sizeof(JSArena);
+    /* Check the net size has the right alignment. */
+    JS_ASSERT(pool->netsize == JS_ARENA_ALIGN(pool, pool->netsize));
 }
 
 JS_PUBLIC_API(void *)
 JS_ArenaAllocate(JSArenaPool *pool, size_t nb)
 {
     /*
      * Search pool from current forward till we find or make enough space.
      *
@@ -93,17 +96,17 @@ JS_ArenaAllocate(JSArenaPool *pool, size
     /*
      * Comparing nb to a->limit looks like a (conceptual) type error, but it's
      * necessary to avoid wrap-around.  Yuk.
      */
     for (a = pool->current; nb > a->limit || a->avail > a->limit - nb; pool->current = a) {
         JSArena **ap = &a->next;
         if (!*ap) {
             /* Not enough space in pool, so we must malloc. */
-            size_t gross = sizeof(JSArena) + JS_MAX(nb, pool->arenasize);
+            size_t gross = sizeof(JSArena) + JS_MAX(nb, pool->netsize);
             a = (JSArena *) OffTheBooks::malloc_(gross);
             if (!a)
                 return NULL;
 
             a->next = NULL;
             a->base = a->avail = jsuword(a) + sizeof(JSArena);
             /*
              * Because malloc returns 8-aligned pointers and sizeof(JSArena) is
@@ -123,18 +126,18 @@ JS_ArenaAllocate(JSArenaPool *pool, size
     a->avail += nb;
     JS_ASSERT(a->base <= a->avail && a->avail <= a->limit);
     return p;
 }
 
 JS_PUBLIC_API(void *)
 JS_ArenaRealloc(JSArenaPool *pool, void *p, size_t size, size_t incr)
 {
-    /* If we've called JS_ArenaRealloc, the new size must be bigger than pool->arenasize. */
-    JS_ASSERT(size + incr > pool->arenasize);
+    /* If we've called JS_ArenaRealloc, the new size must be bigger than pool->netsize. */
+    JS_ASSERT(size + incr > pool->netsize);
 
     /* Find the arena containing |p|. */
     JSArena *a;
     JSArena **ap = &pool->first.next;
     while (true) {
         a = *ap;
         if (JS_IS_IN_ARENA(a, p))
             break;
@@ -172,17 +175,17 @@ JS_PUBLIC_API(void *)
 JS_ArenaGrow(JSArenaPool *pool, void *p, size_t size, size_t incr)
 {
     void *newp;
 
     /*
      * If p points to an oversized allocation, it owns an entire arena, so we
      * can simply realloc the arena.
      */
-    if (size > pool->arenasize)
+    if (size > pool->netsize)
         return JS_ArenaRealloc(pool, p, size, incr);
 
     JS_ARENA_ALLOCATE(newp, pool, size + incr);
     if (newp)
         memcpy(newp, p, size);
     return newp;
 }
 
--- a/js/src/jsarena.h
+++ b/js/src/jsarena.h
@@ -61,17 +61,18 @@ struct JSArena {
     jsuword     base;           /* aligned base address, follows this header */
     jsuword     limit;          /* one beyond last byte in arena */
     jsuword     avail;          /* points to next available byte */
 };
 
 struct JSArenaPool {
     JSArena     first;          /* first arena in pool list */
     JSArena     *current;       /* arena from which to allocate space */
-    size_t      arenasize;      /* net exact size of a new arena */
+    size_t      netsize;        /* net exact size of a new arena; equal to |size| 
+                                   from JS_InitArenaPool minus sizeof(JSArena) */
     jsuword     mask;           /* alignment mask (power-of-2 - 1) */
 };
 
 #define JS_ARENA_ALIGN(pool, n) (((jsuword)(n) + (pool)->mask) & ~(pool)->mask)
 
 #define JS_ARENA_ALLOCATE(p, pool, nb)                                        \
     JS_ARENA_ALLOCATE_CAST(p, void *, pool, nb)
 
@@ -169,17 +170,17 @@ struct JSArenaPool {
         *(pnext) = (a)->next;                                                 \
         JS_CLEAR_ARENA(a);                                                    \
         js::UnwantedForeground::free_(a);                                      \
         (a) = NULL;                                                           \
     JS_END_MACRO
 
 /*
  * Initialize an arena pool with a minimum size per arena of |size| bytes.
- * |align| must be 1, 2, 4 or 8.
+ * |size| must be a power-of-two.  |align| must be 1, 2, 4 or 8.
  */
 extern JS_PUBLIC_API(void)
 JS_InitArenaPool(JSArenaPool *pool, const char *name, size_t size,
                  size_t align);
 
 /*
  * Free the arenas in pool.  The user may continue to allocate from pool
  * after calling this function.  There is no need to call JS_InitArenaPool()
@@ -220,16 +221,18 @@ JS_ArenaGrow(JSArenaPool *pool, void *p,
 
 extern JS_PUBLIC_API(void)
 JS_ArenaRelease(JSArenaPool *pool, char *mark);
 
 JS_END_EXTERN_C
 
 #ifdef __cplusplus
 
+#include "jstl.h"
+
 namespace js {
 
 template <typename T>
 inline T *
 ArenaArray(JSArenaPool &pool, unsigned count)
 {
     void *v;
     JS_ARENA_ALLOCATE(v, &pool, count * sizeof(T));
@@ -302,16 +305,16 @@ ArenaAllocatedSize(const JSArenaPool &po
     return res;
 }
 
 /* Move the contents of oldPool into newPool, and reset oldPool. */
 inline void
 MoveArenaPool(JSArenaPool *oldPool, JSArenaPool *newPool)
 {
     *newPool = *oldPool;
-    JS_InitArenaPool(oldPool, NULL, newPool->arenasize, newPool->mask + 1);
+    JS_InitArenaPool(oldPool, NULL, RoundUpPow2(newPool->netsize), newPool->mask + 1);
 }
 
 } /* namespace js */
 
 #endif /* __cplusplus */
 
 #endif /* jsarena_h___ */
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -311,19 +311,16 @@ js_PurgeThreads(JSContext *cx)
             thread->data.purge(cx);
         }
     }
 #else
     cx->runtime->threadData.purge(cx);
 #endif
 }
 
-static const size_t ARENA_HEADER_SIZE_HACK = 40;
-static const size_t TEMP_POOL_CHUNK_SIZE = 4096 - ARENA_HEADER_SIZE_HACK;
-
 JSContext *
 js_NewContext(JSRuntime *rt, size_t stackChunkSize)
 {
     JSContext *cx;
     JSBool first;
     JSContextCallback cxCallback;
 
     /*
@@ -340,18 +337,18 @@ js_NewContext(JSRuntime *rt, size_t stac
 #if JS_STACK_GROWTH_DIRECTION > 0
     cx->stackLimit = (jsuword) -1;
 #endif
     cx->iterValue.setMagic(JS_NO_ITER_VALUE);
     JS_STATIC_ASSERT(JSVERSION_DEFAULT == 0);
     JS_ASSERT(cx->findVersion() == JSVERSION_DEFAULT);
     VOUCH_DOES_NOT_REQUIRE_STACK();
 
-    JS_InitArenaPool(&cx->tempPool, "temp", TEMP_POOL_CHUNK_SIZE, sizeof(jsdouble));
-    JS_InitArenaPool(&cx->regExpPool, "regExp", TEMP_POOL_CHUNK_SIZE, sizeof(int));
+    JS_InitArenaPool(&cx->tempPool, "temp", 4096, sizeof(jsdouble));
+    JS_InitArenaPool(&cx->regExpPool, "regExp", 4096, sizeof(int));
 
     JS_ASSERT(cx->resolveFlags == 0);
 
     if (!cx->busyArrays.init()) {
         Foreground::delete_(cx);
         return NULL;
     }
 
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -127,20 +127,17 @@ JSCompartment::~JSCompartment()
 }
 
 bool
 JSCompartment::init(JSContext *cx)
 {
     activeAnalysis = activeInference = false;
     types.init(cx);
 
-    /* Duplicated from jscntxt.cpp. :XXX: bug 675150 fix hack. */
-    static const size_t ARENA_HEADER_SIZE_HACK = 40;
-
-    JS_InitArenaPool(&pool, "analysis", 4096 - ARENA_HEADER_SIZE_HACK, 8);
+    JS_InitArenaPool(&pool, "analysis", 4096, 8);
 
     if (!crossCompartmentWrappers.init())
         return false;
 
     if (!scriptFilenameTable.init())
         return false;
 
     regExpAllocator = rt->new_<WTF::BumpPointerAllocator>();