From f91dc93620a331a72953978d6d6b78e7a3964396 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 08:40:38 -0700 Subject: [PATCH 1/7] adjust locking in EntityItem::getActionDataInternal --- libraries/entities/src/EntityItem.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 1fd9acc1e2..701732b921 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1790,8 +1790,16 @@ const QByteArray EntityItem::getActionDataInternal() const { const QByteArray EntityItem::getActionData() const { QByteArray result; assertUnlocked(); + + if (_actionDataDirty) { + EntityItem* unconstThis = const_cast(this); + unconstThis->withWriteLock([&] { + getActionDataInternal(); + }); + } + withReadLock([&] { - result = getActionDataInternal(); + result = _allActionsDataCache; }); return result; } From 2b5b4f111823c4858c78be41aa371144c70fc1c3 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 09:54:16 -0700 Subject: [PATCH 2/7] avoid unneeded read-lock if action-data was dirty --- libraries/entities/src/EntityItem.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 701732b921..08e63fb1d1 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1795,6 +1795,7 @@ const QByteArray EntityItem::getActionData() const { EntityItem* unconstThis = const_cast(this); unconstThis->withWriteLock([&] { getActionDataInternal(); + return _allActionsDataCache; }); } From d504f449e4754b3d88030888d1c48d30e5dc019d Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 09:55:11 -0700 Subject: [PATCH 3/7] undo last commit -- avoid unneeded read-lock if action-data was dirty --- libraries/entities/src/EntityItem.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 08e63fb1d1..701732b921 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1795,7 +1795,6 @@ const QByteArray EntityItem::getActionData() const { EntityItem* unconstThis = const_cast(this); unconstThis->withWriteLock([&] { getActionDataInternal(); - return _allActionsDataCache; }); } From 08a0bf33a49833eb080c42b531634c61ed4e05b7 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 09:55:55 -0700 Subject: [PATCH 4/7] avoid unneeded read-lock if action-data was dirty --- libraries/entities/src/EntityItem.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 701732b921..91ea4864df 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1795,12 +1795,13 @@ const QByteArray EntityItem::getActionData() const { EntityItem* unconstThis = const_cast(this); unconstThis->withWriteLock([&] { getActionDataInternal(); + result = _allActionsDataCache; + }); + } else { + withReadLock([&] { + result = _allActionsDataCache; }); } - - withReadLock([&] { - result = _allActionsDataCache; - }); return result; } From f6a0004f26f73ec7c506ec3d7a287fbd6799f29b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 10:45:24 -0700 Subject: [PATCH 5/7] get rid of some useless consts --- libraries/entities/src/EntityItem.cpp | 4 ++-- libraries/entities/src/EntityItem.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 91ea4864df..4d5fb1b180 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1776,7 +1776,7 @@ void EntityItem::serializeActions(bool& success, QByteArray& result) const { return; } -const QByteArray EntityItem::getActionDataInternal() const { +QByteArray EntityItem::getActionDataInternal() const { if (_actionDataDirty) { bool success; serializeActions(success, _allActionsDataCache); @@ -1787,7 +1787,7 @@ const QByteArray EntityItem::getActionDataInternal() const { return _allActionsDataCache; } -const QByteArray EntityItem::getActionData() const { +QByteArray EntityItem::getActionData() const { QByteArray result; assertUnlocked(); diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 858dc7e326..9e6430b4c3 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -401,7 +401,7 @@ public: bool removeAction(EntitySimulation* simulation, const QUuid& actionID); bool clearActions(EntitySimulation* simulation); void setActionData(QByteArray actionData); - const QByteArray getActionData() const; + QByteArray getActionData() const; bool hasActions() { return !_objectActions.empty(); } QList getActionIDs() { return _objectActions.keys(); } QVariantMap getActionArguments(const QUuid& actionID) const; @@ -415,7 +415,7 @@ public: protected: - const QByteArray getActionDataInternal() const; + QByteArray getActionDataInternal() const; void setActionDataInternal(QByteArray actionData); static bool _sendPhysicsUpdates; From a1096510e80dc566ec9be49411099cc11c1584a2 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 10:52:01 -0700 Subject: [PATCH 6/7] put some useless consts back --- libraries/entities/src/EntityItem.cpp | 4 ++-- libraries/entities/src/EntityItem.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 4d5fb1b180..91ea4864df 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1776,7 +1776,7 @@ void EntityItem::serializeActions(bool& success, QByteArray& result) const { return; } -QByteArray EntityItem::getActionDataInternal() const { +const QByteArray EntityItem::getActionDataInternal() const { if (_actionDataDirty) { bool success; serializeActions(success, _allActionsDataCache); @@ -1787,7 +1787,7 @@ QByteArray EntityItem::getActionDataInternal() const { return _allActionsDataCache; } -QByteArray EntityItem::getActionData() const { +const QByteArray EntityItem::getActionData() const { QByteArray result; assertUnlocked(); diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 9e6430b4c3..858dc7e326 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -401,7 +401,7 @@ public: bool removeAction(EntitySimulation* simulation, const QUuid& actionID); bool clearActions(EntitySimulation* simulation); void setActionData(QByteArray actionData); - QByteArray getActionData() const; + const QByteArray getActionData() const; bool hasActions() { return !_objectActions.empty(); } QList getActionIDs() { return _objectActions.keys(); } QVariantMap getActionArguments(const QUuid& actionID) const; @@ -415,7 +415,7 @@ public: protected: - QByteArray getActionDataInternal() const; + const QByteArray getActionDataInternal() const; void setActionDataInternal(QByteArray actionData); static bool _sendPhysicsUpdates; From 22d6b6df34526da4be267bdafbeeae188c91fdb0 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Wed, 28 Oct 2015 13:49:52 -0700 Subject: [PATCH 7/7] make withWriteLock and withTryWriteLock const --- libraries/entities/src/EntityItem.cpp | 3 +-- libraries/shared/src/shared/ReadWriteLockable.h | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 91ea4864df..f012ba6eee 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1792,8 +1792,7 @@ const QByteArray EntityItem::getActionData() const { assertUnlocked(); if (_actionDataDirty) { - EntityItem* unconstThis = const_cast(this); - unconstThis->withWriteLock([&] { + withWriteLock([&] { getActionDataInternal(); result = _allActionsDataCache; }); diff --git a/libraries/shared/src/shared/ReadWriteLockable.h b/libraries/shared/src/shared/ReadWriteLockable.h index a7d30d562b..98dff4a841 100644 --- a/libraries/shared/src/shared/ReadWriteLockable.h +++ b/libraries/shared/src/shared/ReadWriteLockable.h @@ -14,7 +14,7 @@ class ReadWriteLockable { public: template - bool withWriteLock(F f, bool require = true) { + bool withWriteLock(F f, bool require = true) const { if (!require) { bool result = _lock.tryLockForWrite(); if (result) { @@ -22,7 +22,7 @@ public: _lock.unlock(); } return result; - } + } QWriteLocker locker(&_lock); f(); @@ -30,7 +30,7 @@ public: } template - bool withTryWriteLock(F f) { + bool withTryWriteLock(F f) const { return withWriteLock(f, false); }