Skip to content

Commit

Permalink
Bug 1254618 - modify nsTransactionStack to use nsDeque rather than st…
Browse files Browse the repository at this point in the history
…d::deque; r=ehsan

Using std::deque here causes problems for libc++ builds; TestTXMgr on
OSX 10.6 opt times out when libc++'d std::deque is used.  Running the
test locally shows that the test process consumes gigabytes (!) of
memory and is thus reduced to swapping, rather than making any progress.
libc++'s std::deque also appears to be slightly slower in said test that
even OSX libstdc++'s std::deque.  (Admittedly, this test is artificial.)

Let's sidestep the slowness of libc++'s std::deque by rewriting
nsTransactionStack to use nsDeque rather than std::deque.  Not only does
this change enable OSX 10.6 tests to pass, it also makes TestTXMgr
significantly faster in opt builds: TestTXMgr is anywhere from 25-60%
faster (depending on the platform) than when using std::deque from
libstdc++ or libc++.
  • Loading branch information
froydnj committed Mar 8, 2016
1 parent a226a7c commit 7578bab
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 42 deletions.
71 changes: 34 additions & 37 deletions editor/txmgr/nsTransactionStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,16 @@
#include "nsTransactionStack.h"
#include "nscore.h"

nsTransactionStack::nsTransactionStack(nsTransactionStack::Type aType)
: mType(aType)
class nsTransactionStackDeallocator : public nsDequeFunctor {
virtual void* operator() (void* aObject) {
RefPtr<nsTransactionItem> releaseMe = dont_AddRef(static_cast<nsTransactionItem*>(aObject));
return nullptr;
}
};

nsTransactionStack::nsTransactionStack(Type aType)
: nsDeque(new nsTransactionStackDeallocator())
, mType(aType)
{
}

Expand All @@ -22,88 +30,77 @@ nsTransactionStack::~nsTransactionStack()
}

void
nsTransactionStack::Push(nsTransactionItem *aTransaction)
nsTransactionStack::Push(nsTransactionItem* aTransactionItem)
{
if (!aTransaction) {
if (!aTransactionItem) {
return;
}

// The stack's bottom is the front of the deque, and the top is the back.
RefPtr<nsTransactionItem> item(aTransaction);
RefPtr<nsTransactionItem> item(aTransactionItem);
Push(item.forget());
}

void
nsTransactionStack::Push(already_AddRefed<nsTransactionItem> aTransaction)
nsTransactionStack::Push(already_AddRefed<nsTransactionItem> aTransactionItem)
{
RefPtr<nsTransactionItem> item(aTransaction);
RefPtr<nsTransactionItem> item(aTransactionItem);
if (!item) {
return;
}

// XXX we really want to use emplace_back here, but we don't have a
// C++11 stdlib everywhere.
RefPtr<nsTransactionItem> dummy;
auto pushedItem = mDeque.insert(mDeque.end(), dummy);
*pushedItem = item.forget();
nsDeque::Push(item.forget().take());
}

already_AddRefed<nsTransactionItem>
nsTransactionStack::Pop()
{
if (mDeque.empty()) {
return nullptr;
}
RefPtr<nsTransactionItem> ret = mDeque.back().forget();
mDeque.pop_back();
return ret.forget();
RefPtr<nsTransactionItem> item =
dont_AddRef(static_cast<nsTransactionItem*>(nsDeque::Pop()));
return item.forget();
}

already_AddRefed<nsTransactionItem>
nsTransactionStack::PopBottom()
{
if (mDeque.empty()) {
return nullptr;
}
RefPtr<nsTransactionItem> ret = mDeque.front().forget();
mDeque.pop_front();
return ret.forget();
RefPtr<nsTransactionItem> item =
dont_AddRef(static_cast<nsTransactionItem*>(nsDeque::PopFront()));
return item.forget();
}

already_AddRefed<nsTransactionItem>
nsTransactionStack::Peek()
{
if (mDeque.empty()) {
return nullptr;
}
RefPtr<nsTransactionItem> ret = mDeque.back();
return ret.forget();
RefPtr<nsTransactionItem> item =
static_cast<nsTransactionItem*>(nsDeque::Peek());
return item.forget();
}

already_AddRefed<nsTransactionItem>
nsTransactionStack::GetItem(int32_t aIndex)
{
if (aIndex < 0 || aIndex >= static_cast<int32_t>(mDeque.size())) {
if (aIndex < 0 || aIndex >= static_cast<int32_t>(nsDeque::GetSize())) {
return nullptr;
}
RefPtr<nsTransactionItem> ret = mDeque[aIndex];
return ret.forget();
RefPtr<nsTransactionItem> item =
static_cast<nsTransactionItem*>(nsDeque::ObjectAt(aIndex));
return item.forget();
}

void
nsTransactionStack::Clear()
{
while (!mDeque.empty()) {
RefPtr<nsTransactionItem> tx = mType == FOR_UNDO ? Pop() : PopBottom();
while (GetSize() != 0) {
RefPtr<nsTransactionItem> item =
mType == FOR_UNDO ? Pop() : PopBottom();
}
}

void
nsTransactionStack::DoTraverse(nsCycleCollectionTraversalCallback &cb)
{
int32_t size = mDeque.size();
int32_t size = GetSize();
for (int32_t i = 0; i < size; ++i) {
nsTransactionItem* item = mDeque[i];
nsTransactionItem* item = static_cast<nsTransactionItem*>(nsDeque::ObjectAt(i));
if (item) {
NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "transaction stack mDeque[i]");
cb.NoteNativeChild(item, NS_CYCLE_COLLECTION_PARTICIPANT(nsTransactionItem));
Expand Down
9 changes: 4 additions & 5 deletions editor/txmgr/nsTransactionStack.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
#ifndef nsTransactionStack_h__
#define nsTransactionStack_h__

#include <deque>
#include "nsDeque.h"
#include "nsAutoPtr.h"

class nsCycleCollectionTraversalCallback;
class nsTransactionItem;

class nsTransactionStack
class nsTransactionStack : private nsDeque
{
public:
enum Type { FOR_UNDO, FOR_REDO };
Expand All @@ -27,14 +27,13 @@ class nsTransactionStack
already_AddRefed<nsTransactionItem> Peek();
already_AddRefed<nsTransactionItem> GetItem(int32_t aIndex);
void Clear();
int32_t GetSize() { return mDeque.size(); }
bool IsEmpty() const { return mDeque.empty(); }
int32_t GetSize() const { return static_cast<int32_t>(nsDeque::GetSize()); }
bool IsEmpty() const { return GetSize() == 0; }

void DoUnlink() { Clear(); }
void DoTraverse(nsCycleCollectionTraversalCallback &cb);

private:
std::deque<RefPtr<nsTransactionItem> > mDeque;
const Type mType;
};

Expand Down

0 comments on commit 7578bab

Please sign in to comment.