Bug 493711 Part 1: switch nsCOMArray from nsVoidArray to nsTArray r=sicking
authorNeil Rashbrook <neil@parkwaycc.co.uk>
Wed, 19 Dec 2012 21:33:54 +0000
changeset 116521 21195f52311c61195b9c555279bc734626e7a1c0
parent 116520 27a1c1839d428dd1e89ad4dae30719744142382e
child 116522 f6550d0960c8acbd79546967c493ed1cd6677ec1
child 117363 11f7569992ce60beff4943aeea34260d824c2769
push id24053
push userneil@parkwaycc.co.uk
push dateWed, 19 Dec 2012 21:42:57 +0000
treeherdermozilla-central@21195f52311c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs493711
milestone20.0a1
first release with
nightly linux32
21195f52311c / 20.0a1 / 20121220030912 / files
nightly linux64
21195f52311c / 20.0a1 / 20121220030912 / files
nightly mac
21195f52311c / 20.0a1 / 20121220030912 / files
nightly win32
21195f52311c / 20.0a1 / 20121220030912 / files
nightly win64
21195f52311c / 20.0a1 / 20121220030912 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 493711 Part 1: switch nsCOMArray from nsVoidArray to nsTArray r=sicking
xpcom/glue/nsCOMArray.cpp
xpcom/glue/nsCOMArray.h
--- a/xpcom/glue/nsCOMArray.cpp
+++ b/xpcom/glue/nsCOMArray.cpp
@@ -1,89 +1,151 @@
 /* -*- 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 "nsCOMArray.h"
 #include "nsCOMPtr.h"
 
-static bool ReleaseObjects(void* aElement, void*);
+// This specialization is private to nsCOMArray.
+// It exists solely to automatically zero-out newly created array elements.
+template<>
+class nsTArrayElementTraits<nsISupports*>
+{
+    typedef nsISupports* E;
+public:
+    // Zero out the value
+    static inline void Construct(E *e) {
+        new (static_cast<void *>(e)) E();
+    }
+    // Invoke the copy-constructor in place.
+    template<class A>
+    static inline void Construct(E *e, const A &arg) {
+        new (static_cast<void *>(e)) E(arg);
+    }
+    // Invoke the destructor in place.
+    static inline void Destruct(E *e) {
+        e->~E();
+    }
+};
+
+static void ReleaseObjects(nsTArray<nsISupports*> &aArray);
 
 // implementations of non-trivial methods in nsCOMArray_base
 
-// copy constructor - we can't just memcpy here, because
-// we have to make sure we own our own array buffer, and that each
-// object gets another AddRef()
 nsCOMArray_base::nsCOMArray_base(const nsCOMArray_base& aOther)
 {
     // make sure we do only one allocation
-    mArray.SizeTo(aOther.Count());
+    mArray.SetCapacity(aOther.Count());
     AppendObjects(aOther);
 }
 
 nsCOMArray_base::~nsCOMArray_base()
 {
-  Clear();
+    Clear();
+}
+
+int32_t
+nsCOMArray_base::IndexOf(nsISupports* aObject) const {
+    return mArray.IndexOf(aObject);
 }
 
 int32_t
 nsCOMArray_base::IndexOfObject(nsISupports* aObject) const {
     nsCOMPtr<nsISupports> supports = do_QueryInterface(aObject);
     NS_ENSURE_TRUE(supports, -1);
 
-    int32_t i, count;
+    uint32_t i, count;
     int32_t retval = -1;
-    count = mArray.Count();
+    count = mArray.Length();
     for (i = 0; i < count; ++i) {
-        nsCOMPtr<nsISupports> arrayItem =
-            do_QueryInterface(reinterpret_cast<nsISupports*>(mArray.ElementAt(i)));
+        nsCOMPtr<nsISupports> arrayItem = do_QueryInterface(mArray[i]);
         if (arrayItem == supports) {
             retval = i;
             break;
         }
     }
     return retval;
 }
 
 bool
+nsCOMArray_base::EnumerateForwards(nsBaseArrayEnumFunc aFunc, void* aData) const
+{
+    for (uint32_t index = 0; index < mArray.Length(); index++)
+        if (!(*aFunc)(mArray[index], aData))
+            return false;
+
+    return true;
+}
+
+bool
+nsCOMArray_base::EnumerateBackwards(nsBaseArrayEnumFunc aFunc, void* aData) const
+{
+    for (uint32_t index = mArray.Length(); index--; )
+        if (!(*aFunc)(mArray[index], aData))
+            return false;
+
+    return true;
+}
+
+int
+nsCOMArray_base::nsCOMArrayComparator(const void* aElement1, const void* aElement2, void* aData)
+{
+    nsCOMArrayComparatorContext* ctx = static_cast<nsCOMArrayComparatorContext*>(aData);
+    return (*ctx->mComparatorFunc)(*static_cast<nsISupports* const*>(aElement1),
+                                   *static_cast<nsISupports* const*>(aElement2),
+                                   ctx->mData);
+}
+
+void nsCOMArray_base::Sort(nsBaseArrayComparatorFunc aFunc, void* aData)
+{
+    if (mArray.Length() > 1) {
+        nsCOMArrayComparatorContext ctx = {aFunc, aData};
+        NS_QuickSort(mArray.Elements(), mArray.Length(), sizeof(nsISupports*),
+                     nsCOMArrayComparator, &ctx);
+    }
+}
+
+bool
 nsCOMArray_base::InsertObjectAt(nsISupports* aObject, int32_t aIndex) {
-    bool result = mArray.InsertElementAt(aObject, aIndex);
-    if (result)
-        NS_IF_ADDREF(aObject);
-    return result;
+    if ((uint32_t)aIndex > mArray.Length())
+        return false;
+
+    if (!mArray.InsertElementAt(aIndex, aObject))
+        return false;
+
+    NS_IF_ADDREF(aObject);
+    return true;
 }
 
 bool
 nsCOMArray_base::InsertObjectsAt(const nsCOMArray_base& aObjects, int32_t aIndex) {
-    bool result = mArray.InsertElementsAt(aObjects.mArray, aIndex);
-    if (result) {
-        // need to addref all these
-        int32_t count = aObjects.Count();
-        for (int32_t i = 0; i < count; ++i) {
-            NS_IF_ADDREF(aObjects.ObjectAt(i));
-        }
-    }
-    return result;
+    if ((uint32_t)aIndex > mArray.Length())
+        return false;
+
+    if (!mArray.InsertElementsAt(aIndex, aObjects.mArray))
+        return false;
+
+    // need to addref all these
+    int32_t count = aObjects.Count();
+    for (int32_t i = 0; i < count; ++i)
+        NS_IF_ADDREF(aObjects.ObjectAt(i));
+
+    return true;
 }
 
 bool
 nsCOMArray_base::ReplaceObjectAt(nsISupports* aObject, int32_t aIndex)
 {
-    // its ok if oldObject is null here
-    nsISupports *oldObject =
-        reinterpret_cast<nsISupports*>(mArray.SafeElementAt(aIndex));
-
-    bool result = mArray.ReplaceElementAt(aObject, aIndex);
-
-    // ReplaceElementAt could fail, such as if the array grows
-    // so only release the existing object if the replacement succeeded
+    bool result = mArray.EnsureLengthAtLeast(aIndex + 1);
     if (result) {
+        nsISupports *oldObject = mArray[aIndex];
         // Make sure to addref first, in case aObject == oldObject
-        NS_IF_ADDREF(aObject);
+        NS_IF_ADDREF(mArray[aIndex] = aObject);
         NS_IF_RELEASE(oldObject);
     }
     return result;
 }
 
 bool
 nsCOMArray_base::RemoveObject(nsISupports *aObject)
 {
@@ -91,76 +153,75 @@ nsCOMArray_base::RemoveObject(nsISupport
     if (result)
         NS_IF_RELEASE(aObject);
     return result;
 }
 
 bool
 nsCOMArray_base::RemoveObjectAt(int32_t aIndex)
 {
-    if (uint32_t(aIndex) < uint32_t(Count())) {
-        nsISupports* element = ObjectAt(aIndex);
+    if (uint32_t(aIndex) < mArray.Length()) {
+        nsISupports* element = mArray[aIndex];
 
-        bool result = mArray.RemoveElementAt(aIndex);
+        mArray.RemoveElementAt(aIndex);
         NS_IF_RELEASE(element);
-        return result;
+        return true;
     }
 
     return false;
 }
 
 bool
 nsCOMArray_base::RemoveObjectsAt(int32_t aIndex, int32_t aCount)
 {
-    if (uint32_t(aIndex) + uint32_t(aCount) <= uint32_t(Count())) {
-        nsVoidArray elementsToDestroy(aCount);
-        for (int32_t i = 0; i < aCount; ++i) {
-            elementsToDestroy.InsertElementAt(mArray.FastElementAt(aIndex + i), i);
-        }
-        bool result = mArray.RemoveElementsAt(aIndex, aCount);
-        for (int32_t i = 0; i < aCount; ++i) {
-            nsISupports* element = static_cast<nsISupports*> (elementsToDestroy.FastElementAt(i));
-            NS_IF_RELEASE(element);
-        }
-        return result;
+    if (uint32_t(aIndex) + uint32_t(aCount) <= mArray.Length()) {
+        nsTArray<nsISupports*> elementsToDestroy(aCount);
+        elementsToDestroy.AppendElements(mArray.Elements() + aIndex, aCount);
+        mArray.RemoveElementsAt(aIndex, aCount);
+        ReleaseObjects(elementsToDestroy);
+        return true;
     }
 
     return false;
 }
 
 // useful for destructors
-bool
-ReleaseObjects(void* aElement, void*)
+void
+ReleaseObjects(nsTArray<nsISupports*> &aArray)
 {
-    nsISupports* element = static_cast<nsISupports*>(aElement);
-    NS_IF_RELEASE(element);
-    return true;
+    for (uint32_t i = 0; i < aArray.Length(); i++)
+        NS_IF_RELEASE(aArray[i]);
 }
 
 void
 nsCOMArray_base::Clear()
 {
-    nsAutoVoidArray objects;
-    objects = mArray;
-    mArray.Clear();
-    objects.EnumerateForwards(ReleaseObjects, nullptr);
+    nsTArray<nsISupports*> objects;
+    objects.SwapElements(mArray);
+    ReleaseObjects(objects);
 }
 
 bool
 nsCOMArray_base::SetCount(int32_t aNewCount)
 {
     NS_ASSERTION(aNewCount >= 0,"SetCount(negative index)");
     if (aNewCount < 0)
-      return false;
+        return false;
 
-    int32_t count = Count(), i;
-    nsAutoVoidArray objects;
-    if (count > aNewCount) {
-        objects.SetCount(count - aNewCount);
-        for (i = aNewCount; i < count; ++i) {
-            objects.ReplaceElementAt(ObjectAt(i), i - aNewCount);
-        }
-    }
-    bool result = mArray.SetCount(aNewCount);
-    objects.EnumerateForwards(ReleaseObjects, nullptr);
-    return result;
+    int32_t count = mArray.Length();
+    if (count > aNewCount)
+        RemoveObjectsAt(aNewCount, mArray.Length() - aNewCount);
+    return mArray.SetLength(aNewCount);
 }
 
+size_t
+nsCOMArray_base::SizeOfExcludingThis(
+                   nsBaseArraySizeOfElementIncludingThisFunc aSizeOfElementIncludingThis,
+                   nsMallocSizeOfFun aMallocSizeOf, void* aData) const
+{
+    size_t n = mArray.SizeOfExcludingThis(aMallocSizeOf);
+
+    if (aSizeOfElementIncludingThis)
+        for (uint32_t index = 0; index < mArray.Length(); index++)
+            n += aSizeOfElementIncludingThis(mArray[index], aMallocSizeOf, aData);
+
+    return n;
+}
--- a/xpcom/glue/nsCOMArray.h
+++ b/xpcom/glue/nsCOMArray.h
@@ -5,115 +5,121 @@
 
 #ifndef nsCOMArray_h__
 #define nsCOMArray_h__
 
 #include "mozilla/Attributes.h"
 
 #include "nsCycleCollectionNoteChild.h"
 #include "nsVoidArray.h"
+#include "nsTArray.h"
 #include "nsISupports.h"
 
 // See below for the definition of nsCOMArray<T>
 
 // a class that's nsISupports-specific, so that we can contain the
 // work of this class in the XPCOM dll
 class NS_COM_GLUE nsCOMArray_base
 {
     friend class nsArray;
 protected:
     nsCOMArray_base() {}
     nsCOMArray_base(int32_t aCount) : mArray(aCount) {}
     nsCOMArray_base(const nsCOMArray_base& other);
     ~nsCOMArray_base();
 
-    int32_t IndexOf(nsISupports* aObject) const {
-        return mArray.IndexOf(aObject);
-    }
-
+    int32_t IndexOf(nsISupports* aObject) const;
     int32_t IndexOfObject(nsISupports* aObject) const;
 
-    bool EnumerateForwards(nsVoidArrayEnumFunc aFunc, void* aData) {
-        return mArray.EnumerateForwards(aFunc, aData);
-    }
+    typedef bool (* nsBaseArrayEnumFunc)
+        (void* aElement, void *aData);
     
-    bool EnumerateBackwards(nsVoidArrayEnumFunc aFunc, void* aData) {
-        return mArray.EnumerateBackwards(aFunc, aData);
-    }
+    // enumerate through the array with a callback.
+    bool EnumerateForwards(nsBaseArrayEnumFunc aFunc, void* aData) const;
     
-    void Sort(nsVoidArrayComparatorFunc aFunc, void* aData) {
-        mArray.Sort(aFunc, aData);
-    }
-    
-    // any method which is not a direct forward to mArray should
-    // avoid inline bodies, so that the compiler doesn't inline them
-    // all over the place
-    void Clear();
+    bool EnumerateBackwards(nsBaseArrayEnumFunc aFunc, void* aData) const;
+
+    typedef int (* nsBaseArrayComparatorFunc)
+        (nsISupports* aElement1, nsISupports* aElement2, void* aData);
+
+    struct nsCOMArrayComparatorContext {
+        nsBaseArrayComparatorFunc mComparatorFunc;
+        void* mData;
+    };
+
+    static int nsCOMArrayComparator(const void* aElement1, const void* aElement2, void* aData);
+    void Sort(nsBaseArrayComparatorFunc aFunc, void* aData);
+
     bool InsertObjectAt(nsISupports* aObject, int32_t aIndex);
     bool InsertObjectsAt(const nsCOMArray_base& aObjects, int32_t aIndex);
     bool ReplaceObjectAt(nsISupports* aObject, int32_t aIndex);
     bool AppendObject(nsISupports *aObject) {
         return InsertObjectAt(aObject, Count());
     }
     bool AppendObjects(const nsCOMArray_base& aObjects) {
         return InsertObjectsAt(aObjects, Count());
     }
     bool RemoveObject(nsISupports *aObject);
-    bool RemoveObjectAt(int32_t aIndex);
-    bool RemoveObjectsAt(int32_t aIndex, int32_t aCount);
 
 public:
-    // override nsVoidArray stuff so that they can be accessed by
-    // consumers of nsCOMArray
+    // elements in the array (including null elements!)
     int32_t Count() const {
-        return mArray.Count();
+        return mArray.Length();
     }
+
     // If the array grows, the newly created entries will all be null;
     // if the array shrinks, the excess entries will all be released.
     bool SetCount(int32_t aNewCount);
 
+    // remove all elements in the array, and call NS_RELEASE on each one
+    void Clear();
+
     nsISupports* ObjectAt(int32_t aIndex) const {
-        return static_cast<nsISupports*>(mArray.FastElementAt(aIndex));
+        return mArray[aIndex];
     }
     
     nsISupports* SafeObjectAt(int32_t aIndex) const {
-        return static_cast<nsISupports*>(mArray.SafeElementAt(aIndex));
+        return mArray.SafeElementAt(aIndex, nullptr);
     }
 
     nsISupports* operator[](int32_t aIndex) const {
         return ObjectAt(aIndex);
     }
 
+    // remove an element at a specific position, shrinking the array
+    // as necessary
+    bool RemoveObjectAt(int32_t aIndex);
+
+    // remove a range of elements at a specific position, shrinking the array
+    // as necessary
+    bool RemoveObjectsAt(int32_t aIndex, int32_t aCount);
+
     // Ensures there is enough space to store a total of aCapacity objects.
     // This method never deletes any objects.
     bool SetCapacity(uint32_t aCapacity) {
-      return aCapacity > 0 ? mArray.SizeTo(static_cast<int32_t>(aCapacity))
-                           : true;
+      return mArray.SetCapacity(aCapacity);
     }
 
+    typedef size_t (* nsBaseArraySizeOfElementIncludingThisFunc)
+        (nsISupports* aElement, nsMallocSizeOfFun aMallocSizeOf, void *aData);
+
     // Measures the size of the array's element storage, and if
     // |aSizeOfElement| is non-NULL, measures the size of things pointed to by
     // elements.
     size_t SizeOfExcludingThis(
-             nsVoidArraySizeOfElementIncludingThisFunc aSizeOfElementIncludingThis,
-             nsMallocSizeOfFun aMallocSizeOf, void* aData = NULL) const {
-        return mArray.SizeOfExcludingThis(aSizeOfElementIncludingThis,
-                                          aMallocSizeOf, aData);
-    }
-    
+             nsBaseArraySizeOfElementIncludingThisFunc aSizeOfElementIncludingThis,
+             nsMallocSizeOfFun aMallocSizeOf, void* aData = NULL) const;
+
 private:
     
     // the actual storage
-    nsVoidArray mArray;
+    nsTArray<nsISupports*> mArray;
 
     // don't implement these, defaults will muck with refcounts!
     nsCOMArray_base& operator=(const nsCOMArray_base& other) MOZ_DELETE;
-
-    // needs to call Clear() which is protected
-    friend void ImplCycleCollectionUnlink(nsCOMArray_base& aField);
 };
 
 inline void
 ImplCycleCollectionUnlink(nsCOMArray_base& aField)
 {
     aField.Clear();
 }
 
@@ -206,51 +212,38 @@ class nsCOMArray : public nsCOMArray_bas
     }
 
     // replaces an existing element. Warning: if the array grows,
     // the newly created entries will all be null
     bool ReplaceObjectAt(T* aObject, int32_t aIndex) {
         return nsCOMArray_base::ReplaceObjectAt(static_cast<nsISupports*>(aObject), aIndex);
     }
 
-    // override nsVoidArray stuff so that they can be accessed by
-    // other methods
-
-    // elements in the array (including null elements!)
-    int32_t Count() const {
-        return nsCOMArray_base::Count();
-    }
-
-    // remove all elements in the array, and call NS_RELEASE on each one
-    void Clear() {
-        nsCOMArray_base::Clear();
-    }
-
     // Enumerator callback function. Return false to stop
     // Here's a more readable form:
     // bool enumerate(T* aElement, void* aData)
     typedef bool (* nsCOMArrayEnumFunc)
         (T* aElement, void *aData);
     
     // enumerate through the array with a callback. 
     bool EnumerateForwards(nsCOMArrayEnumFunc aFunc, void* aData) {
-        return nsCOMArray_base::EnumerateForwards(nsVoidArrayEnumFunc(aFunc),
+        return nsCOMArray_base::EnumerateForwards(nsBaseArrayEnumFunc(aFunc),
                                                   aData);
     }
 
     bool EnumerateBackwards(nsCOMArrayEnumFunc aFunc, void* aData) {
-        return nsCOMArray_base::EnumerateBackwards(nsVoidArrayEnumFunc(aFunc),
+        return nsCOMArray_base::EnumerateBackwards(nsBaseArrayEnumFunc(aFunc),
                                                   aData);
     }
     
     typedef int (* nsCOMArrayComparatorFunc)
         (T* aElement1, T* aElement2, void* aData);
         
     void Sort(nsCOMArrayComparatorFunc aFunc, void* aData) {
-        nsCOMArray_base::Sort(nsVoidArrayComparatorFunc(aFunc), aData);
+        nsCOMArray_base::Sort(nsBaseArrayComparatorFunc(aFunc), aData);
     }
 
     // append an object, growing the array as necessary
     bool AppendObject(T *aObject) {
         return nsCOMArray_base::AppendObject(static_cast<nsISupports*>(aObject));
     }
 
     // append objects, growing the array as necessary
@@ -260,39 +253,27 @@ class nsCOMArray : public nsCOMArray_bas
     
     // remove the first instance of the given object and shrink the
     // array as necessary
     // Warning: if you pass null here, it will remove the first null element
     bool RemoveObject(T *aObject) {
         return nsCOMArray_base::RemoveObject(static_cast<nsISupports*>(aObject));
     }
 
-    // remove an element at a specific position, shrinking the array
-    // as necessary
-    bool RemoveObjectAt(int32_t aIndex) {
-        return nsCOMArray_base::RemoveObjectAt(aIndex);
-    }
-
-    // remove a range of elements at a specific position, shrinking the array
-    // as necessary
-    bool RemoveObjectsAt(int32_t aIndex, int32_t aCount) {
-        return nsCOMArray_base::RemoveObjectsAt(aIndex, aCount);
-    }
-
     // Each element in an nsCOMArray<T> is actually a T*, so this function is
     // "IncludingThis" rather than "ExcludingThis" because it needs to measure
     // the memory taken by the T itself as well as anything it points to.
     typedef size_t (* nsCOMArraySizeOfElementIncludingThisFunc)
         (T* aElement, nsMallocSizeOfFun aMallocSizeOf, void *aData);
     
     size_t SizeOfExcludingThis(
              nsCOMArraySizeOfElementIncludingThisFunc aSizeOfElementIncludingThis, 
              nsMallocSizeOfFun aMallocSizeOf, void *aData = NULL) const {
         return nsCOMArray_base::SizeOfExcludingThis(
-                 nsVoidArraySizeOfElementIncludingThisFunc(aSizeOfElementIncludingThis),
+                 nsBaseArraySizeOfElementIncludingThisFunc(aSizeOfElementIncludingThis),
                  aMallocSizeOf, aData);
     }
 
 private:
 
     // don't implement these!
     nsCOMArray<T>& operator=(const nsCOMArray<T>& other) MOZ_DELETE;
 };