Bug 847248 (part 10) - Rewrite TimerEventAllocator and remove nsFixedSizeAllocator. r=bz,ehsan; sr=bsmedberg.

--HG--
extra : rebase_source : aaa9c2d0ffd268e8f1b9184e8230b1e7932f014d

xpcom/ds/Makefile.in

@@ -23,7 +23,6 @@ CPPSRCS		= \
 		nsAtomService.cpp \
 		nsByteBuffer.cpp \
 		nsCRT.cpp \
-		nsFixedSizeAllocator.cpp \
 		nsHashPropertyBag.cpp \
 		nsHashtable.cpp \
 		nsINIParserImpl.cpp \
@@ -71,7 +70,6 @@ EXPORTS		= \
 		nsCppSharedAllocator.h \
 		nsCRT.h \
 		nsExpirationTracker.h \
-		nsFixedSizeAllocator.h \
 		nsHashtable.h \
 		nsIByteBuffer.h \
 		nsIUnicharBuffer.h \

xpcom/ds/nsFixedSizeAllocator.cpp

@@ -1,113 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "nsDebug.h"
-#include "nsFixedSizeAllocator.h"
-
-nsFixedSizeAllocator::Bucket *
-nsFixedSizeAllocator::AddBucket(size_t aSize)
-{
-    void* p;
-    PL_ARENA_ALLOCATE(p, &mPool, sizeof(Bucket));
-    if (! p)
-        return nullptr;
-
-    Bucket* bucket = static_cast<Bucket*>(p);
-    bucket->mSize  = aSize;
-    bucket->mFirst = nullptr;
-    bucket->mNext  = mBuckets;
-
-    mBuckets = bucket;
-    return bucket;
-}
-
-nsresult
-nsFixedSizeAllocator::Init(const char* aName,
-                           const size_t* aBucketSizes,
-                           int32_t aNumBuckets,
-                           int32_t aChunkSize,
-                           int32_t aAlign)
-{
-    NS_PRECONDITION(aNumBuckets > 0, "no buckets");
-    if (aNumBuckets <= 0)
-        return NS_ERROR_INVALID_ARG;
-
-    // Blow away the old pool if we're being re-initialized.
-    if (mBuckets)
-        PL_FinishArenaPool(&mPool);
-
-    PL_InitArenaPool(&mPool, aName, aChunkSize, aAlign);
-
-    mBuckets = nullptr;
-    for (int32_t i = 0; i < aNumBuckets; ++i)
-        AddBucket(aBucketSizes[i]);
-
-    return NS_OK;
-}
-
-nsFixedSizeAllocator::Bucket *
-nsFixedSizeAllocator::FindBucket(size_t aSize)
-{
-    Bucket** link = &mBuckets;
-    Bucket* bucket;
-
-    while ((bucket = *link) != nullptr) {
-        if (aSize == bucket->mSize) {
-            // Promote to the head of the list, under the assumption
-            // that we'll allocate same-sized object contemporaneously.
-            *link = bucket->mNext;
-            bucket->mNext = mBuckets;
-            mBuckets = bucket;
-            return bucket;
-        }
-
-        link = &bucket->mNext;
-    }
-    return nullptr;
-}
-
-void*
-nsFixedSizeAllocator::Alloc(size_t aSize)
-{
-    Bucket* bucket = FindBucket(aSize);
-    if (! bucket) {
-        // Oops, we don't carry that size. Let's fix that.
-        bucket = AddBucket(aSize);
-        if (! bucket)
-            return nullptr;
-    }
-
-    void* next;
-    if (bucket->mFirst) {
-        next = bucket->mFirst;
-        bucket->mFirst = bucket->mFirst->mNext;
-    }
-    else {
-        PL_ARENA_ALLOCATE(next, &mPool, aSize);
-        if (!next)
-            return nullptr;
-    }
-
-#ifdef DEBUG
-    memset(next, 0xc8, aSize);
-#endif
-
-    return next;
-}
-
-void
-nsFixedSizeAllocator::Free(void* aPtr, size_t aSize)
-{
-    FreeEntry* entry = reinterpret_cast<FreeEntry*>(aPtr);
-    Bucket* bucket = FindBucket(aSize);
-
-#ifdef DEBUG
-    NS_ASSERTION(bucket && bucket->mSize == aSize, "ack! corruption! bucket->mSize != aSize!");
-    memset(aPtr, 0xd8, bucket->mSize);
-#endif
-
-    entry->mNext = bucket->mFirst;
-    bucket->mFirst = entry;
-}

xpcom/ds/nsFixedSizeAllocator.h

@@ -1,193 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-/*
-  A simple fixed-size allocator that allocates its memory from an
-  arena.
-
-  WARNING: you probably shouldn't use this class.  If you are thinking of using
-  it, you should have measurements that indicate it has a clear advantage over
-  vanilla malloc/free or vanilla new/delete.
-
-  This allocator has the following notable properties.
-
-  - Although it can handle blocks of any size, its performance will degrade
-    rapidly if used to allocate blocks of many different sizes.  Ideally, it
-    should be used to allocate and recycle many fixed-size blocks.
-
-  - None of the chunks allocated are released back to the OS unless Init() is
-    re-called or the nsFixedSizeAllocator is destroyed.  So it's generally a
-    bad choice if it might live for a long time (e.g. see bug 847210).
-
-  - You have to manually construct and destruct objects allocated with it.
-    Furthermore, any objects that haven't been freed when the allocator is
-    destroyed won't have their destructors run.  So if you are allocating
-    objects that have destructors they should all be manually freed (and
-    destructed) before the allocator is destroyed.
-
-  - It does no locking and so is not thread-safe.  If all allocations and
-    deallocations are on a single thread, this is fine and these operations
-    might be faster than vanilla malloc/free due to the lack of locking.
-
-    Otherwise, you need to add locking yourself.  In unusual circumstances,
-    this can be a good thing, because it reduces contention over the main
-    malloc/free lock.  See TimerEventAllocator and bug 733277 for an example.
-
-  - Because it's an arena-style allocator, it might reduce fragmentation,
-    because objects allocated with the arena won't be co-allocated with
-    longer-lived objects.  However, this is hard to demonstrate and you should
-    not assume the effects are significant without conclusive measurements.
-
-  Here is a typical usage pattern.  Note that no locking is done in this
-  example so it's only safe for use on a single thread.
-
-    #include NEW_H // You'll need this!
-    #include "nsFixedSizeAllocator.h"
-
-    // Say this is the object you want to allocate a ton of
-    class Foo {
-    public:
-      // Implement placement new & delete operators that will
-      // use the fixed size allocator.
-      static Foo *
-      Create(nsFixedSizeAllocator &aAllocator)
-      {
-        void *place = aAllocator.Alloc(sizeof(Foo));
-        return place ? ::new (place) Foo() : nullptr;
-      }
-
-      static void
-      Destroy(nsFixedSizeAllocator &aAllocator, Foo *aFoo)
-      {
-        aFoo->~Foo();
-        aAllocator.Free(aFoo, sizeof(Foo));
-      }
-
-      // ctor & dtor
-      Foo() {}
-      ~Foo() {}
-    };
-
-    int main(int argc, char* argv[])
-    {
-      // Somewhere in your code, you'll need to create an
-      // nsFixedSizeAllocator object and initialize it:
-      nsFixedSizeAllocator pool;
-
-      // The fixed size allocator will support multiple fixed sizes.
-      // This array lists an initial set of sizes that the allocator
-      // should be prepared to support. In our case, there's just one,
-      // which is Foo.
-      static const size_t kBucketSizes[] = { sizeof(Foo) }
-
-      // This is the number of different "buckets" you'll need for
-      // fixed size objects. In our example, this will be "1".
-      static const int32_t kNumBuckets = sizeof(kBucketSizes) / sizeof(size_t);
-
-      // This is the size of the chunks used by the allocator, which should be
-      // a power of two to minimize slop.
-      static const int32_t kInitialPoolSize = 4096;
-
-      // Initialize (or re-initialize) the pool.
-      pool.Init("TheFooPool", kBucketSizes, kNumBuckets, kInitialPoolSize);
-
-      // Create a new Foo object using the pool:
-      Foo* foo = Foo::Create(pool);
-      if (!foo) {
-        // uh oh, out of memory!
-      }
-
-      // Delete the object. The memory used by `foo' is recycled in
-      // the pool, and placed in a freelist.
-      Foo::Destroy(foo);
-
-      // Create another foo: this one will be allocated from the
-      // free-list.
-      foo = Foo::Create(pool);
-
-      // When pool is destroyed, all of its memory is automatically
-      // freed. N.B. it will *not* call your objects' destructors! In
-      // this case, foo's ~Foo() method would never be called.
-    }
-*/
-
-#ifndef nsFixedSizeAllocator_h__
-#define nsFixedSizeAllocator_h__
-
-#include "nscore.h"
-#include "nsError.h"
-#include "plarena.h"
-
-class nsFixedSizeAllocator
-{
-protected:
-    PLArenaPool mPool;
-
-    struct Bucket;
-    struct FreeEntry;
-  
-    friend struct Bucket;
-    friend struct FreeEntry;
-
-    struct FreeEntry {
-        FreeEntry* mNext;
-    };
-
-    struct Bucket {
-        size_t     mSize;
-        FreeEntry* mFirst;
-        Bucket*    mNext;
-    };
-
-    Bucket* mBuckets;
-
-    Bucket *
-    AddBucket(size_t aSize);
-
-    Bucket *
-    FindBucket(size_t aSize);
-
-public:
-    nsFixedSizeAllocator() : mBuckets(nullptr) {}
-
-    ~nsFixedSizeAllocator() {
-        if (mBuckets)
-            PL_FinishArenaPool(&mPool);
-    }
-
-    /**
-     * Initialize the fixed size allocator.
-     * - 'aName' is used to tag the underlying PLArena object for debugging and
-     *   measurement purposes.
-     * - 'aNumBuckets' specifies the number of elements in 'aBucketSizes'.
-     * - 'aBucketSizes' is an array of integral block sizes that this allocator
-     *   should be prepared to handle.
-     * - 'aChunkSize' is the size of the chunks used.  It should be a power of
-     *   two to minimize slop bytes caused by the underlying malloc
-     *   implementation rounding up request sizes.  Some of the space in each
-     *   chunk will be used by the nsFixedSizeAllocator (or the underlying
-     *   PLArena) itself.
-     */
-    nsresult
-    Init(const char* aName,
-         const size_t* aBucketSizes,
-         int32_t aNumBuckets,
-         int32_t aChunkSize,
-         int32_t aAlign = 0);
-
-    /**
-     * Allocate a block of memory 'aSize' bytes big.
-     */
-    void* Alloc(size_t aSize);
-
-    /**
-     * Free a pointer allocated using a fixed-size allocator
-     */
-    void Free(void* aPtr, size_t aSize);
-};
-
-
-
-#endif // nsFixedSizeAllocator_h__

xpcom/threads/nsTimerImpl.cpp

@@ -1,5 +1,6 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
@@ -8,9 +9,8 @@
 #include "nsAutoPtr.h"
 #include "nsThreadManager.h"
 #include "nsThreadUtils.h"
+#include "plarena.h"
 #include "sampler.h"
-#include NEW_H
-#include "nsFixedSizeAllocator.h"
 
 using mozilla::TimeDuration;
 using mozilla::TimeStamp;
@@ -57,36 +57,49 @@ myNS_MeanAndStdDev(double n, double sumOfValues, double sumOfSquaredValues,
 
 namespace {
 
-// TimerEventAllocator is a fixed size allocator class which is used in order
-// to avoid the default allocator lock contention when firing timer events.
-// It is a thread-safe wrapper around nsFixedSizeAllocator.  The thread-safety
-// is required because nsTimerEvent objects are allocated on the timer thread,
-// and freed on another thread.  Since this is a TimerEventAllocator specific
-// lock, the lock contention issue is only limited to the allocation and
-// deallocation of nsTimerEvent objects.
-class TimerEventAllocator : public nsFixedSizeAllocator {
+// TimerEventAllocator is a thread-safe allocator used only for nsTimerEvents.
+// It's needed to avoid contention over the default allocator lock when
+// firing timer events (see bug 733277).  The thread-safety is required because
+// nsTimerEvent objects are allocated on the timer thread, and freed on another
+// thread.  Because TimerEventAllocator has its own lock, contention over that
+// lock is limited to the allocation and deallocation of nsTimerEvent objects.
+//
+// Because this allocator is layered over PLArenaPool, it never shrinks -- even
+// "freed" nsTimerEvents aren't truly freed, they're just put onto a free-list
+// for later recycling.  So the amount of memory consumed will always be equal
+// to the high-water mark consumption.  But nsTimerEvents are small and it's
+// unusual to have more than a few hundred of them, so this shouldn't be a
+// problem in practice.
+
+class TimerEventAllocator
+{
+private:
+  struct FreeEntry {
+    FreeEntry* mNext;
+  };
+
+  PLArenaPool mPool;
+  FreeEntry* mFirstFree;
+  mozilla::Monitor mMonitor;
+
 public:
-    TimerEventAllocator() :
+  TimerEventAllocator()
+    : mFirstFree(nullptr),
       mMonitor("TimerEventAllocator")
   {
+    PL_InitArenaPool(&mPool, "TimerEventPool", 4096, /* align = */ 0);
   }
 
-  void* Alloc(size_t aSize)
+  ~TimerEventAllocator()
   {
-    mozilla::MonitorAutoLock lock(mMonitor);
-    return nsFixedSizeAllocator::Alloc(aSize);
-  }
-  void Free(void* aPtr, size_t aSize)
-  {
-    mozilla::MonitorAutoLock lock(mMonitor);
-    nsFixedSizeAllocator::Free(aPtr, aSize);
+    PL_FinishArenaPool(&mPool);
   }
 
-private:
-  mozilla::Monitor mMonitor;
+  void* Alloc(size_t aSize);
+  void Free(void* aPtr);
 };
 
-}
+} // anonymous namespace
 
 class nsTimerEvent : public nsRunnable {
 public:
@@ -115,7 +128,7 @@ public:
     return sAllocator->Alloc(size);
   }
   void operator delete(void* p) {
-    sAllocator->Free(p, sizeof(nsTimerEvent));
+    sAllocator->Free(p);
     DeleteAllocatorIfNeeded();
   }
 
@@ -145,6 +158,40 @@ TimerEventAllocator* nsTimerEvent::sAllocator = nullptr;
 int32_t nsTimerEvent::sAllocatorUsers = 0;
 bool nsTimerEvent::sCanDeleteAllocator = false;
 
+namespace {
+
+void* TimerEventAllocator::Alloc(size_t aSize)
+{
+  MOZ_ASSERT(aSize == sizeof(nsTimerEvent));
+
+  mozilla::MonitorAutoLock lock(mMonitor);
+
+  void* p;
+  if (mFirstFree) {
+    p = mFirstFree;
+    mFirstFree = mFirstFree->mNext;
+  }
+  else {
+    PL_ARENA_ALLOCATE(p, &mPool, aSize);
+    if (!p)
+      return nullptr;
+  }
+
+  return p;
+}
+
+void TimerEventAllocator::Free(void* aPtr)
+{
+  mozilla::MonitorAutoLock lock(mMonitor);
+
+  FreeEntry* entry = reinterpret_cast<FreeEntry*>(aPtr);
+
+  entry->mNext = mFirstFree;
+  mFirstFree = entry;
+}
+
+} // anonymous namespace
+
 NS_IMPL_THREADSAFE_QUERY_INTERFACE1(nsTimerImpl, nsITimer)
 NS_IMPL_THREADSAFE_ADDREF(nsTimerImpl)
 
@@ -547,10 +594,6 @@ void nsTimerImpl::Fire()
 void nsTimerEvent::Init()
 {
   sAllocator = new TimerEventAllocator();
-  static const size_t kBucketSizes[] = {sizeof(nsTimerEvent)};
-  static const int32_t kNumBuckets = mozilla::ArrayLength(kBucketSizes);
-  static const int32_t kInitialPoolSize = 4096;
-  sAllocator->Init("TimerEventPool", kBucketSizes, kNumBuckets, kInitialPoolSize);
 }
 
 void nsTimerEvent::Shutdown()