Skip to content

Commit

Permalink
Optimize|libdeng2|Lockable: Use automatic storage with Lockable
Browse files Browse the repository at this point in the history
Lockable, as used by Guard, should not use the current pimpl idiom
implementation as this allocates storage for the private instance
from the heap.

The multithreaded logging system makes frequent use of Guards for
ensuring concurrency robustness. This inadvertently resulted in a
significant and unnecessary perf bottleneck.
  • Loading branch information
danij-deng committed Apr 9, 2014
1 parent 6ace3b0 commit 7e863a4
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 27 deletions.
8 changes: 6 additions & 2 deletions doomsday/libdeng2/include/de/concurrency/lockable.h
@@ -1,7 +1,8 @@
/*
* The Doomsday Engine Project -- libdeng2
*
* Copyright © 2004-2013 Jaakko Keränen <jaakko.keranen@iki.fi>
* @authors Copyright © 2004-2014 Jaakko Keränen <jaakko.keranen@iki.fi>
* @authors Copyright © 2014 Daniel Swanson <danij@dengine.net>
*
* @par License
* LGPL: http://www.gnu.org/licenses/lgpl.html
Expand Down Expand Up @@ -50,7 +51,10 @@ class DENG2_PUBLIC Lockable
bool isLocked() const;

private:
DENG2_PRIVATE(d)
mutable QMutex _mutex;

mutable int _lockCount;
mutable QMutex _countMutex;
};

} // namespace de
Expand Down
42 changes: 17 additions & 25 deletions doomsday/libdeng2/src/concurrency/lockable.cpp
@@ -1,7 +1,8 @@
/*
* The Doomsday Engine Project -- libdeng2
*
* Copyright © 2004-2013 Jaakko Keränen <jaakko.keranen@iki.fi>
* @authors Copyright © 2004-2014 Jaakko Keränen <jaakko.keranen@iki.fi>
* @authors Copyright © 2014 Daniel Swanson <danij@dengine.net>
*
* @par License
* LGPL: http://www.gnu.org/licenses/lgpl.html
Expand All @@ -22,18 +23,9 @@

namespace de {

DENG2_PIMPL_NOREF(Lockable)
{
mutable QMutex mutex;

mutable int lockCount;
mutable QMutex countMutex;

Instance() : mutex(QMutex::Recursive), lockCount(0)
{}
};

Lockable::Lockable() : d(new Instance)
Lockable::Lockable()
: _mutex(QMutex::Recursive)
, _lockCount(0)
{}

Lockable::~Lockable()
Expand All @@ -46,31 +38,31 @@ Lockable::~Lockable()

void Lockable::lock() const
{
d->countMutex.lock();
d->lockCount++;
d->countMutex.unlock();
_countMutex.lock();
_lockCount++;
_countMutex.unlock();

d->mutex.lock();
_mutex.lock();
}

void Lockable::unlock() const
{
// Release the lock.
d->mutex.unlock();
_mutex.unlock();

d->countMutex.lock();
d->lockCount--;
d->countMutex.unlock();
_countMutex.lock();
_lockCount--;
_countMutex.unlock();

DENG2_ASSERT(d->lockCount >= 0);
DENG2_ASSERT(_lockCount >= 0);
}

bool Lockable::isLocked() const
{
bool result;
d->countMutex.lock();
result = (d->lockCount > 0);
d->countMutex.unlock();
_countMutex.lock();
result = (_lockCount > 0);
_countMutex.unlock();
return result;
}

Expand Down

0 comments on commit 7e863a4

Please sign in to comment.