From d2315c15a12077ea681cc9049b7c9123a5038a7d Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Feb 2017 11:13:05 -0800
Subject: [PATCH] avoid duplicate parse of AvatarEntityData entries

---
 interface/src/avatar/Avatar.cpp | 55 ++++++++++++++++++++++++++++-----
 interface/src/avatar/Avatar.h   | 10 ++++++
 2 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp
index ad3b615549..0b59100bcb 100644
--- a/interface/src/avatar/Avatar.cpp
+++ b/interface/src/avatar/Avatar.cpp
@@ -231,13 +231,40 @@ void Avatar::updateAvatarEntities() {
     QScriptEngine scriptEngine;
     entityTree->withWriteLock([&] {
         AvatarEntityMap avatarEntities = getAvatarEntityData();
-        for (auto entityID : avatarEntities.keys()) {
+        AvatarEntityMap::const_iterator dataItr = avatarEntities.begin();
+        while (dataItr != avatarEntities.end()) {
+            // compute hash of data.  TODO? cache this?
+            QByteArray data = dataItr.value();
+            uint32_t newHash = qHash(data);
+
+            // check to see if we recognize this hash and whether it was already successfully processed
+            QUuid entityID = dataItr.key();
+            MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID);
+            if (stateItr != _avatarEntityDataHashes.end()) {
+                if (stateItr.value().success) {
+                    if (newHash == stateItr.value().hash) {
+                        // data hasn't changed --> nothing to do
+                        ++dataItr;
+                        continue;
+                    }
+                } else {
+                    // NOTE: if the data was unsuccessful in producing an entity in the past
+                    // we will try again just in case something changed (unlikely).
+                    // Unfortunately constantly trying to build the entity for this data costs
+                    // CPU cycles that we'd rather not spend.
+                    // TODO? put a maximum number of tries on this?
+                }
+            } else {
+                // remember this hash for the future
+                stateItr = _avatarEntityDataHashes.insert(entityID, AvatarEntityDataHash(newHash));
+            }
+            ++dataItr;
+
             // see EntityEditPacketSender::queueEditEntityMessage for the other end of this.  unpack properties
             // and either add or update the entity.
-            QByteArray jsonByteArray = avatarEntities.value(entityID);
-            QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(jsonByteArray);
+            QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(data);
             if (!jsonProperties.isObject()) {
-                qCDebug(interfaceapp) << "got bad avatarEntity json" << QString(jsonByteArray.toHex());
+                qCDebug(interfaceapp) << "got bad avatarEntity json" << QString(data.toHex());
                 continue;
             }
 
@@ -265,18 +292,22 @@ void Avatar::updateAvatarEntities() {
                 properties.setScript(noScript);
             }
 
+            // try to build the entity
             EntityItemPointer entity = entityTree->findEntityByEntityItemID(EntityItemID(entityID));
-
-            // NOTE: we don't bother checking whether updateEntity or addEntity succeed here:
-            // if not we'll try again next time the data is changed.  This is to close a performance DOS vector
-            // whereby invalid entity data is given to the AvatarMixer and constant retries kill performance.
+            bool success = true;
             if (entity) {
                 if (entityTree->updateEntity(entityID, properties)) {
                     entity->updateLastEditedFromRemote();
+                } else {
+                    success = false;
                 }
             } else {
                 entity = entityTree->addEntity(entityID, properties);
+                if (!entity) {
+                    success = false;
+                }
             }
+            stateItr.value().success = success;
         }
 
         AvatarEntityIDs recentlyDettachedAvatarEntities = getAndClearRecentlyDetachedIDs();
@@ -289,6 +320,14 @@ void Avatar::updateAvatarEntities() {
                     }
                 }
             });
+
+            // remove stale data hashes
+            foreach (auto entityID, recentlyDettachedAvatarEntities) {
+                MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID);
+                if (stateItr != _avatarEntityDataHashes.end()) {
+                    _avatarEntityDataHashes.erase(stateItr);
+                }
+            }
         }
     });
 
diff --git a/interface/src/avatar/Avatar.h b/interface/src/avatar/Avatar.h
index 5c05702e92..80d387fd33 100644
--- a/interface/src/avatar/Avatar.h
+++ b/interface/src/avatar/Avatar.h
@@ -269,6 +269,16 @@ protected:
 
 
 private:
+    class AvatarEntityDataHash {
+    public:
+        AvatarEntityDataHash(uint32_t h) : hash(h) {};
+        uint32_t hash { 0 };
+        bool success { false };
+    };
+
+    using MapOfAvatarEntityDataHashes = QMap<QUuid, AvatarEntityDataHash>;
+    MapOfAvatarEntityDataHashes _avatarEntityDataHashes;
+
     uint64_t _lastRenderUpdateTime { 0 };
     int _leftPointerGeometryID { 0 };
     int _rightPointerGeometryID { 0 };