From 17ab0cb92e2c2aedf35e5b5d363acd779c01c9f8 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 8 Feb 2019 13:12:10 -0800
Subject: [PATCH 1/9] more robust ShapeManager garbage collector

---
 libraries/physics/src/ShapeManager.cpp | 11 +++++++++--
 libraries/physics/src/ShapeManager.h   |  2 +-
 libraries/shared/src/HashKey.h         |  3 +++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/libraries/physics/src/ShapeManager.cpp b/libraries/physics/src/ShapeManager.cpp
index ef7a4a1749..cb37b0f96d 100644
--- a/libraries/physics/src/ShapeManager.cpp
+++ b/libraries/physics/src/ShapeManager.cpp
@@ -57,7 +57,14 @@ bool ShapeManager::releaseShapeByKey(const HashKey& key) {
         if (shapeRef->refCount > 0) {
             shapeRef->refCount--;
             if (shapeRef->refCount == 0) {
-                _pendingGarbage.push_back(key);
+                // look for existing entry in _pendingGarbage, starting from the back
+                for (int32_t i = _pendingGarbage.size() - 1; i > -1; --i) {
+                    if (_pendingGarbage[i] == key.getHash64()) {
+                        // already on the list, don't add it again
+                        return true;
+                    }
+                }
+                _pendingGarbage.push_back(key.getHash64());
                 const int MAX_SHAPE_GARBAGE_CAPACITY = 255;
                 if (_pendingGarbage.size() > MAX_SHAPE_GARBAGE_CAPACITY) {
                     collectGarbage();
@@ -89,7 +96,7 @@ bool ShapeManager::releaseShape(const btCollisionShape* shape) {
 void ShapeManager::collectGarbage() {
     int numShapes = _pendingGarbage.size();
     for (int i = 0; i < numShapes; ++i) {
-        HashKey& key = _pendingGarbage[i];
+        HashKey key(_pendingGarbage[i]);
         ShapeReference* shapeRef = _shapeMap.find(key);
         if (shapeRef && shapeRef->refCount == 0) {
             ShapeFactory::deleteShape(shapeRef->shape);
diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h
index d75bb1dc4a..58f1b53c18 100644
--- a/libraries/physics/src/ShapeManager.h
+++ b/libraries/physics/src/ShapeManager.h
@@ -75,7 +75,7 @@ private:
 
     // btHashMap is required because it supports memory alignment of the btCollisionShapes
     btHashMap<HashKey, ShapeReference> _shapeMap;
-    btAlignedObjectArray<HashKey> _pendingGarbage;
+    btAlignedObjectArray<uint64_t> _pendingGarbage;
 };
 
 #endif // hifi_ShapeManager_h
diff --git a/libraries/shared/src/HashKey.h b/libraries/shared/src/HashKey.h
index 5fce182084..a32a8664a7 100644
--- a/libraries/shared/src/HashKey.h
+++ b/libraries/shared/src/HashKey.h
@@ -32,6 +32,9 @@ class HashKey {
 public:
     static float getNumQuantizedValuesPerMeter();
 
+    HashKey() {}
+    HashKey(uint64_t hash) : _hash(hash) {}
+
     // These two methods are required by btHashMap.
     bool equals(const HashKey& other) const { return _hash == other._hash; }
     int32_t getHash() const { return (int32_t)((uint32_t)_hash); }

From a3567dea3a2db620262f503a770fe750812dbf47 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 8 Feb 2019 14:18:33 -0800
Subject: [PATCH 2/9] clean HashKey API, reduce dependency tree

---
 libraries/physics/src/ShapeManager.cpp | 21 +++++------
 libraries/physics/src/ShapeManager.h   |  4 +--
 libraries/shared/src/HashKey.h         |  5 ++-
 libraries/shared/src/ShapeInfo.cpp     | 49 ++++++++++++++------------
 libraries/shared/src/ShapeInfo.h       |  6 ++--
 5 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/libraries/physics/src/ShapeManager.cpp b/libraries/physics/src/ShapeManager.cpp
index cb37b0f96d..991cd0efc9 100644
--- a/libraries/physics/src/ShapeManager.cpp
+++ b/libraries/physics/src/ShapeManager.cpp
@@ -33,8 +33,8 @@ const btCollisionShape* ShapeManager::getShape(const ShapeInfo& info) {
     if (info.getType() == SHAPE_TYPE_NONE) {
         return nullptr;
     }
-    HashKey key = info.getHash();
-    ShapeReference* shapeRef = _shapeMap.find(key);
+    HashKey hashKey(info.getHash());
+    ShapeReference* shapeRef = _shapeMap.find(hashKey);
     if (shapeRef) {
         shapeRef->refCount++;
         return shapeRef->shape;
@@ -44,27 +44,28 @@ const btCollisionShape* ShapeManager::getShape(const ShapeInfo& info) {
         ShapeReference newRef;
         newRef.refCount = 1;
         newRef.shape = shape;
-        newRef.key = key;
-        _shapeMap.insert(key, newRef);
+        newRef.key = info.getHash();
+        _shapeMap.insert(hashKey, newRef);
     }
     return shape;
 }
 
 // private helper method
-bool ShapeManager::releaseShapeByKey(const HashKey& key) {
-    ShapeReference* shapeRef = _shapeMap.find(key);
+bool ShapeManager::releaseShapeByKey(uint64_t key) {
+    HashKey hashKey(key);
+    ShapeReference* shapeRef = _shapeMap.find(hashKey);
     if (shapeRef) {
         if (shapeRef->refCount > 0) {
             shapeRef->refCount--;
             if (shapeRef->refCount == 0) {
                 // look for existing entry in _pendingGarbage, starting from the back
                 for (int32_t i = _pendingGarbage.size() - 1; i > -1; --i) {
-                    if (_pendingGarbage[i] == key.getHash64()) {
+                    if (_pendingGarbage[i] == key) {
                         // already on the list, don't add it again
                         return true;
                     }
                 }
-                _pendingGarbage.push_back(key.getHash64());
+                _pendingGarbage.push_back(key);
                 const int MAX_SHAPE_GARBAGE_CAPACITY = 255;
                 if (_pendingGarbage.size() > MAX_SHAPE_GARBAGE_CAPACITY) {
                     collectGarbage();
@@ -107,8 +108,8 @@ void ShapeManager::collectGarbage() {
 }
 
 int ShapeManager::getNumReferences(const ShapeInfo& info) const {
-    HashKey key = info.getHash();
-    const ShapeReference* shapeRef = _shapeMap.find(key);
+    HashKey hashKey(info.getHash());
+    const ShapeReference* shapeRef = _shapeMap.find(hashKey);
     if (shapeRef) {
         return shapeRef->refCount;
     }
diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h
index 58f1b53c18..47b6c5340a 100644
--- a/libraries/physics/src/ShapeManager.h
+++ b/libraries/physics/src/ShapeManager.h
@@ -63,13 +63,13 @@ public:
     bool hasShape(const btCollisionShape* shape) const;
 
 private:
-    bool releaseShapeByKey(const HashKey& key);
+    bool releaseShapeByKey(uint64_t key);
 
     class ShapeReference {
     public:
         int refCount;
         const btCollisionShape* shape;
-        HashKey key;
+        uint64_t key { 0 };
         ShapeReference() : refCount(0), shape(nullptr) {}
     };
 
diff --git a/libraries/shared/src/HashKey.h b/libraries/shared/src/HashKey.h
index a32a8664a7..446eb4c25f 100644
--- a/libraries/shared/src/HashKey.h
+++ b/libraries/shared/src/HashKey.h
@@ -39,13 +39,12 @@ public:
     bool equals(const HashKey& other) const { return _hash == other._hash; }
     int32_t getHash() const { return (int32_t)((uint32_t)_hash); }
 
-    void clear() { _hash = _hashCount = 0; }
-    bool isNull() const { return _hash == 0 && _hashCount == 0; }
+    // These methods for accumulating a hash.
     void hashUint64(uint64_t data);
     void hashFloat(float data);
     void hashVec3(const glm::vec3& data);
 
-    uint64_t getHash64() const { return _hash; } // for debug/test purposes
+    uint64_t getHash64() const { return _hash; }
 
 private:
     uint64_t _hash { 0 };
diff --git a/libraries/shared/src/ShapeInfo.cpp b/libraries/shared/src/ShapeInfo.cpp
index c256cf2b76..bf51e455c5 100644
--- a/libraries/shared/src/ShapeInfo.cpp
+++ b/libraries/shared/src/ShapeInfo.cpp
@@ -13,6 +13,7 @@
 
 #include <math.h>
 
+#include "HashKey.h"
 #include "NumericalConstants.h" // for MILLIMETERS_PER_METER
 
 /**jsdoc
@@ -96,7 +97,7 @@ void ShapeInfo::clear() {
     _sphereCollection.clear();
     _halfExtents = glm::vec3(0.0f);
     _offset = glm::vec3(0.0f);
-    _hashKey.clear();
+    _hash64 = 0;
     _type = SHAPE_TYPE_NONE;
 }
 
@@ -131,14 +132,14 @@ void ShapeInfo::setParams(ShapeType type, const glm::vec3& halfExtents, QString
         default:
             break;
     }
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 void ShapeInfo::setBox(const glm::vec3& halfExtents) {
     _url = "";
     _type = SHAPE_TYPE_BOX;
     setHalfExtents(halfExtents);
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 void ShapeInfo::setSphere(float radius) {
@@ -146,7 +147,7 @@ void ShapeInfo::setSphere(float radius) {
     _type = SHAPE_TYPE_SPHERE;
     radius = glm::max(radius, MIN_HALF_EXTENT);
     _halfExtents = glm::vec3(radius);
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 void ShapeInfo::setMultiSphere(const std::vector<glm::vec3>& centers, const std::vector<float>& radiuses) {
@@ -158,12 +159,12 @@ void ShapeInfo::setMultiSphere(const std::vector<glm::vec3>& centers, const std:
         SphereData sphere = SphereData(centers[i], radiuses[i]);
         _sphereCollection.push_back(sphere);
     }
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 void ShapeInfo::setPointCollection(const ShapeInfo::PointCollection& pointCollection) {
     _pointCollection = pointCollection;
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 void ShapeInfo::setCapsuleY(float radius, float cylinderHalfHeight) {
@@ -172,12 +173,12 @@ void ShapeInfo::setCapsuleY(float radius, float cylinderHalfHeight) {
     radius = glm::max(radius, MIN_HALF_EXTENT);
     cylinderHalfHeight = glm::max(cylinderHalfHeight, 0.0f);
     _halfExtents = glm::vec3(radius, cylinderHalfHeight + radius, radius);
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 void ShapeInfo::setOffset(const glm::vec3& offset) {
     _offset = offset;
-    _hashKey.clear();
+    _hash64 = 0;
 }
 
 uint32_t ShapeInfo::getNumSubShapes() const {
@@ -269,20 +270,21 @@ float ShapeInfo::computeVolume() const {
     return volume;
 }
 
-const HashKey& ShapeInfo::getHash() const {
+uint64_t ShapeInfo::getHash() const {
     // NOTE: we cache the key so we only ever need to compute it once for any valid ShapeInfo instance.
-    if (_hashKey.isNull() && _type != SHAPE_TYPE_NONE) {
+    if (_hash64 == 0 && _type != SHAPE_TYPE_NONE) {
+        HashKey hashKey;
         // The key is not yet cached therefore we must compute it.
 
-        _hashKey.hashUint64((uint64_t)_type);
+        hashKey.hashUint64((uint64_t)_type);
         if (_type == SHAPE_TYPE_MULTISPHERE) {
             for (auto &sphereData : _sphereCollection) {
-                _hashKey.hashVec3(glm::vec3(sphereData));
-                _hashKey.hashFloat(sphereData.w);
+                hashKey.hashVec3(glm::vec3(sphereData));
+                hashKey.hashFloat(sphereData.w);
             }
         } else if (_type != SHAPE_TYPE_SIMPLE_HULL) {
-            _hashKey.hashVec3(_halfExtents);
-            _hashKey.hashVec3(_offset);
+            hashKey.hashVec3(_halfExtents);
+            hashKey.hashVec3(_offset);
         } else {
             // TODO: we could avoid hashing all of these points if we were to supply the ShapeInfo with a unique
             // descriptive string.  Shapes that are uniquely described by their type and URL could just put their
@@ -292,7 +294,7 @@ const HashKey& ShapeInfo::getHash() const {
             const int numPoints = (int)points.size();
 
             for (int i = 0; i < numPoints; ++i) {
-                _hashKey.hashVec3(points[i]);
+                hashKey.hashVec3(points[i]);
             }
         }
 
@@ -300,23 +302,24 @@ const HashKey& ShapeInfo::getHash() const {
         if (!url.isEmpty()) {
             QByteArray baUrl = url.toLocal8Bit();
             uint32_t urlHash = qChecksum(baUrl.data(), baUrl.size());
-            _hashKey.hashUint64((uint64_t)urlHash);
+            hashKey.hashUint64((uint64_t)urlHash);
         }
 
         if (_type == SHAPE_TYPE_COMPOUND || _type == SHAPE_TYPE_SIMPLE_COMPOUND) {
             uint64_t numHulls = (uint64_t)_pointCollection.size();
-            _hashKey.hashUint64(numHulls);
+            hashKey.hashUint64(numHulls);
         } else if (_type == SHAPE_TYPE_MULTISPHERE) {
             uint64_t numSpheres = (uint64_t)_sphereCollection.size();
-            _hashKey.hashUint64(numSpheres);
+            hashKey.hashUint64(numSpheres);
         } else if (_type == SHAPE_TYPE_SIMPLE_HULL) {
-            _hashKey.hashUint64(1);
-        } 
+            hashKey.hashUint64(1);
+        }
+        _hash64 = hashKey.getHash64();
     }
-    return _hashKey;
+    return _hash64;
 }
 
 void ShapeInfo::setHalfExtents(const glm::vec3& halfExtents) {
     _halfExtents = glm::max(halfExtents, glm::vec3(MIN_HALF_EXTENT));
-    _hashKey.clear();
+    _hash64 = 0;
 }
diff --git a/libraries/shared/src/ShapeInfo.h b/libraries/shared/src/ShapeInfo.h
index d838d7b214..6b0f981b24 100644
--- a/libraries/shared/src/ShapeInfo.h
+++ b/libraries/shared/src/ShapeInfo.h
@@ -18,8 +18,6 @@
 #include <glm/glm.hpp>
 #include <glm/gtx/norm.hpp>
 
-#include "HashKey.h"
-
 const float MIN_SHAPE_OFFSET = 0.001f; // offsets less than 1mm will be ignored
 
 // Bullet has a mesh generation util for convex shapes that we used to
@@ -91,7 +89,7 @@ public:
 
     float computeVolume() const;
 
-    const HashKey& getHash() const;
+    uint64_t getHash() const;
 
 protected:
     void setHalfExtents(const glm::vec3& halfExtents);
@@ -102,7 +100,7 @@ protected:
     TriangleIndices _triangleIndices;
     glm::vec3 _halfExtents = glm::vec3(0.0f);
     glm::vec3 _offset = glm::vec3(0.0f);
-    mutable HashKey _hashKey;
+    mutable uint64_t _hash64;
     ShapeType _type = SHAPE_TYPE_NONE;
 };
 

From d7f38bac17c9945e3eda395c366cf037fbf5ba27 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 8 Feb 2019 14:50:30 -0800
Subject: [PATCH 3/9] fix ShapeInfoTests to use reduced HashKey API

---
 tests/physics/src/ShapeInfoTests.cpp | 36 ++++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/tests/physics/src/ShapeInfoTests.cpp b/tests/physics/src/ShapeInfoTests.cpp
index efc88a4032..0116eb027f 100644
--- a/tests/physics/src/ShapeInfoTests.cpp
+++ b/tests/physics/src/ShapeInfoTests.cpp
@@ -55,20 +55,20 @@ void ShapeInfoTests::testHashFunctions() {
         // test sphere
         info.setSphere(radiusX);
         ++testCount;
-        HashKey key = info.getHash();
-        hashPtr = hashes.find(key);
+        HashKey hashKey(info.getHash());
+        hashPtr = hashes.find(hashKey);
         if (hashPtr) {
             std::cout << testCount << "  hash collision sphere radius = " << radiusX
-                << "  h = 0x" << std::hex << key.getHash() << " : 0x" << *hashPtr
+                << "  h = 0x" << std::hex << hashKey.getHash() << " : 0x" << *hashPtr
                 << std::dec << std::endl;
             ++numCollisions;
             assert(false);
         } else {
-            hashes.insert(key, key.getHash());
+            hashes.insert(hashKey, hashKey.getHash());
         }
         // track bit distribution counts to evaluate hash function randomness
         for (int j = 0; j < NUM_HASH_BITS; ++j) {
-            if (masks[j] & key.getHash()) {
+            if (masks[j] & hashKey.getHash()) {
                 ++bits[j];
             }
         }
@@ -80,21 +80,21 @@ void ShapeInfoTests::testHashFunctions() {
                 // test box
                 info.setBox(glm::vec3(radiusX, radiusY, radiusZ));
                 ++testCount;
-                HashKey key = info.getHash();
-                hashPtr = hashes.find(key);
+                HashKey hashKey(info.getHash());
+                hashPtr = hashes.find(hashKey);
                 if (hashPtr) {
                     std::cout << testCount << "  hash collision box dimensions = < " << radiusX
                         << ", " << radiusY << ", " << radiusZ << " >"
-                        << "  h = 0x" << std::hex << key.getHash() << " : 0x" << *hashPtr << " : 0x" << key.getHash64()
+                        << "  h = 0x" << std::hex << hashKey.getHash() << " : 0x" << *hashPtr << " : 0x" << hashKey.getHash64()
                         << std::dec << std::endl;
                     ++numCollisions;
                     assert(false);
                 } else {
-                    hashes.insert(key, key.getHash());
+                    hashes.insert(hashKey, hashKey.getHash());
                 }
                 // track bit distribution counts to evaluate hash function randomness
                 for (int k = 0; k < NUM_HASH_BITS; ++k) {
-                    if (masks[k] & key.getHash()) {
+                    if (masks[k] & hashKey.getHash()) {
                         ++bits[k];
                     }
                 }
@@ -117,14 +117,14 @@ void ShapeInfoTests::testBoxShape() {
     ShapeInfo info;
     glm::vec3 halfExtents(1.23f, 4.56f, 7.89f);
     info.setBox(halfExtents);
-    HashKey key = info.getHash();
+    HashKey hashKey(info.getHash());
 
     const btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
     QCOMPARE(shape != nullptr, true);
 
     ShapeInfo otherInfo = info;
     HashKey otherKey = otherInfo.getHash();
-    QCOMPARE(key.getHash(), otherKey.getHash());
+    QCOMPARE(hashKey.getHash(), otherKey.getHash());
 
     delete shape;
 }
@@ -133,14 +133,14 @@ void ShapeInfoTests::testSphereShape() {
     ShapeInfo info;
     float radius = 1.23f;
     info.setSphere(radius);
-    HashKey key = info.getHash();
+    HashKey hashKey = info.getHash();
 
     const btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
     QCOMPARE(shape != nullptr, true);
 
     ShapeInfo otherInfo = info;
     HashKey otherKey = otherInfo.getHash();
-    QCOMPARE(key.getHash(), otherKey.getHash());
+    QCOMPARE(hashKey.getHash(), otherKey.getHash());
 
     delete shape;
 }
@@ -151,14 +151,14 @@ void ShapeInfoTests::testCylinderShape() {
     float radius = 1.23f;
     float height = 4.56f;
     info.setCylinder(radius, height);
-    HashKey key = info.getHash();
+    HashKey hashKey(info.getHash());
 
     btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
     QCOMPARE(shape != nullptr, true);
 
     ShapeInfo otherInfo = info;
     HashKey otherKey = otherInfo.getHash();
-    QCOMPARE(key.getHash(), otherKey.getHash());
+    QCOMPARE(hashKey.getHash(), otherKey.getHash());
 
     delete shape;
     */
@@ -170,14 +170,14 @@ void ShapeInfoTests::testCapsuleShape() {
     float radius = 1.23f;
     float height = 4.56f;
     info.setCapsule(radius, height);
-    HashKey key = info.getHash();
+    HashKey hashKey(info.getHash());
 
     btCollisionShape* shape = ShapeFactory::createShapeFromInfo(info);
     QCOMPARE(shape != nullptr, true);
 
     ShapeInfo otherInfo = info;
     HashKey otherKey = otherInfo.getHash();
-    QCOMPARE(key.getHash(), otherKey.getHash());
+    QCOMPARE(hashKey.getHash(), otherKey.getHash());
 
     delete shape;
     */

From c9b142531bbdbcc8b63669d70ee19d4e10eaf087 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Sat, 9 Feb 2019 08:21:24 -0800
Subject: [PATCH 4/9] add more tracing details in PrePhysics

---
 libraries/physics/src/ShapeManager.cpp | 36 +++++++++++++++++++-------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/libraries/physics/src/ShapeManager.cpp b/libraries/physics/src/ShapeManager.cpp
index 991cd0efc9..ad937d2bcc 100644
--- a/libraries/physics/src/ShapeManager.cpp
+++ b/libraries/physics/src/ShapeManager.cpp
@@ -17,7 +17,10 @@
 
 #include "ShapeFactory.h"
 
+const int MAX_RING_SIZE = 256;
+
 ShapeManager::ShapeManager() {
+    _garbageRing.reserve(MAX_RING_SIZE);
 }
 
 ShapeManager::~ShapeManager() {
@@ -58,17 +61,29 @@ bool ShapeManager::releaseShapeByKey(uint64_t key) {
         if (shapeRef->refCount > 0) {
             shapeRef->refCount--;
             if (shapeRef->refCount == 0) {
-                // look for existing entry in _pendingGarbage, starting from the back
-                for (int32_t i = _pendingGarbage.size() - 1; i > -1; --i) {
-                    if (_pendingGarbage[i] == key) {
+                // look for existing entry in _garbageRing
+                int32_t ringSize = _garbageRing.size();
+                for (int32_t i = 0; i < ringSize; ++i) {
+                    int32_t j = (_ringIndex + ringSize) % ringSize;
+                    if (_garbageRing[j] == key) {
                         // already on the list, don't add it again
                         return true;
                     }
                 }
-                _pendingGarbage.push_back(key);
-                const int MAX_SHAPE_GARBAGE_CAPACITY = 255;
-                if (_pendingGarbage.size() > MAX_SHAPE_GARBAGE_CAPACITY) {
-                    collectGarbage();
+                if (ringSize == MAX_RING_SIZE) {
+                    // remove one
+                    HashKey hashKeyToRemove(_garbageRing[_ringIndex]);
+                    ShapeReference* shapeRef = _shapeMap.find(hashKeyToRemove);
+                    if (shapeRef && shapeRef->refCount == 0) {
+                        ShapeFactory::deleteShape(shapeRef->shape);
+                        _shapeMap.remove(hashKeyToRemove);
+                    }
+                    // replace at _ringIndex and advance
+                    _garbageRing[_ringIndex] = key;
+                    _ringIndex = (_ringIndex + 1) % ringSize;
+                } else {
+                    // add one
+                    _garbageRing.push_back(key);
                 }
             }
             return true;
@@ -95,16 +110,17 @@ bool ShapeManager::releaseShape(const btCollisionShape* shape) {
 }
 
 void ShapeManager::collectGarbage() {
-    int numShapes = _pendingGarbage.size();
+    int numShapes = _garbageRing.size();
     for (int i = 0; i < numShapes; ++i) {
-        HashKey key(_pendingGarbage[i]);
+        HashKey key(_garbageRing[i]);
         ShapeReference* shapeRef = _shapeMap.find(key);
         if (shapeRef && shapeRef->refCount == 0) {
             ShapeFactory::deleteShape(shapeRef->shape);
             _shapeMap.remove(key);
         }
     }
-    _pendingGarbage.clear();
+    _ringIndex = 0;
+    _garbageRing.clear();
 }
 
 int ShapeManager::getNumReferences(const ShapeInfo& info) const {

From 8bdac589b88f3339bf7f4a4f9f4c1017a4cd296f Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Sat, 9 Feb 2019 08:23:09 -0800
Subject: [PATCH 5/9] use RingBuffer for gradual garbage collection

---
 interface/src/Application.cpp        | 59 +++++++++++++++++-----------
 libraries/physics/src/ShapeManager.h |  8 +++-
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 1029398794..b3c978df9a 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -6309,40 +6309,53 @@ void Application::update(float deltaTime) {
                 PROFILE_RANGE(simulation_physics, "PrePhysics");
                 PerformanceTimer perfTimer("prePhysics)");
                 {
+                    PROFILE_RANGE(simulation_physics, "RemoveEntities");
                     const VectorOfMotionStates& motionStates = _entitySimulation->getObjectsToRemoveFromPhysics();
                     _physicsEngine->removeObjects(motionStates);
                     _entitySimulation->deleteObjectsRemovedFromPhysics();
                 }
 
-                VectorOfMotionStates motionStates;
-                getEntities()->getTree()->withReadLock([&] {
-                    _entitySimulation->getObjectsToAddToPhysics(motionStates);
-                    _physicsEngine->addObjects(motionStates);
-
-                });
-                getEntities()->getTree()->withReadLock([&] {
-                    _entitySimulation->getObjectsToChange(motionStates);
-                    VectorOfMotionStates stillNeedChange = _physicsEngine->changeObjects(motionStates);
-                    _entitySimulation->setObjectsToChange(stillNeedChange);
-                });
+                {
+                    PROFILE_RANGE(simulation_physics, "AddEntities");
+                    VectorOfMotionStates motionStates;
+                    getEntities()->getTree()->withReadLock([&] {
+                        _entitySimulation->getObjectsToAddToPhysics(motionStates);
+                        _physicsEngine->addObjects(motionStates);
+                    });
+                }
+                {
+                    VectorOfMotionStates motionStates;
+                    PROFILE_RANGE(simulation_physics, "ChangeEntities");
+                    getEntities()->getTree()->withReadLock([&] {
+                        _entitySimulation->getObjectsToChange(motionStates);
+                        VectorOfMotionStates stillNeedChange = _physicsEngine->changeObjects(motionStates);
+                        _entitySimulation->setObjectsToChange(stillNeedChange);
+                    });
+                }
 
                 _entitySimulation->applyDynamicChanges();
 
                 t1 = std::chrono::high_resolution_clock::now();
 
-                PhysicsEngine::Transaction transaction;
-                avatarManager->buildPhysicsTransaction(transaction);
-                _physicsEngine->processTransaction(transaction);
-                avatarManager->handleProcessedPhysicsTransaction(transaction);
-                myAvatar->getCharacterController()->buildPhysicsTransaction(transaction);
-                _physicsEngine->processTransaction(transaction);
-                myAvatar->getCharacterController()->handleProcessedPhysicsTransaction(transaction);
-                myAvatar->prepareForPhysicsSimulation();
-                _physicsEngine->enableGlobalContactAddedCallback(myAvatar->isFlying());
+                {
+                    PROFILE_RANGE(simulation_physics, "Avatars");
+                    PhysicsEngine::Transaction transaction;
+                    avatarManager->buildPhysicsTransaction(transaction);
+                    _physicsEngine->processTransaction(transaction);
+                    avatarManager->handleProcessedPhysicsTransaction(transaction);
+                    myAvatar->getCharacterController()->buildPhysicsTransaction(transaction);
+                    _physicsEngine->processTransaction(transaction);
+                    myAvatar->getCharacterController()->handleProcessedPhysicsTransaction(transaction);
+                    myAvatar->prepareForPhysicsSimulation();
+                    _physicsEngine->enableGlobalContactAddedCallback(myAvatar->isFlying());
+                }
 
-                _physicsEngine->forEachDynamic([&](EntityDynamicPointer dynamic) {
-                    dynamic->prepareForPhysicsSimulation();
-                });
+                {
+                    PROFILE_RANGE(simulation_physics, "PrepareActions");
+                    _physicsEngine->forEachDynamic([&](EntityDynamicPointer dynamic) {
+                        dynamic->prepareForPhysicsSimulation();
+                    });
+                }
             }
             auto t2 = std::chrono::high_resolution_clock::now();
             {
diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h
index 47b6c5340a..7583b7d919 100644
--- a/libraries/physics/src/ShapeManager.h
+++ b/libraries/physics/src/ShapeManager.h
@@ -12,6 +12,8 @@
 #ifndef hifi_ShapeManager_h
 #define hifi_ShapeManager_h
 
+#include <vector>
+
 #include <btBulletDynamicsCommon.h>
 #include <LinearMath/btHashMap.h>
 
@@ -41,6 +43,7 @@
 // later.  When that list grows big enough the ShapeManager will remove any matching
 // entries that still have zero ref-count.
 
+
 class ShapeManager {
 public:
 
@@ -75,7 +78,10 @@ private:
 
     // btHashMap is required because it supports memory alignment of the btCollisionShapes
     btHashMap<HashKey, ShapeReference> _shapeMap;
-    btAlignedObjectArray<uint64_t> _pendingGarbage;
+    //btAlignedObjectArray<uint64_t> _pendingGarbage;
+
+    std::vector<uint64_t> _garbageRing;
+    uint32_t _ringIndex { 0 };
 };
 
 #endif // hifi_ShapeManager_h

From d4c52e4b1c2fbef3b31664383a665510ebd9fd68 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Mon, 11 Feb 2019 09:16:31 -0800
Subject: [PATCH 6/9] fix implicit cast warning for Visual Studio

---
 libraries/physics/src/ShapeManager.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libraries/physics/src/ShapeManager.cpp b/libraries/physics/src/ShapeManager.cpp
index ad937d2bcc..8acbe51540 100644
--- a/libraries/physics/src/ShapeManager.cpp
+++ b/libraries/physics/src/ShapeManager.cpp
@@ -62,7 +62,7 @@ bool ShapeManager::releaseShapeByKey(uint64_t key) {
             shapeRef->refCount--;
             if (shapeRef->refCount == 0) {
                 // look for existing entry in _garbageRing
-                int32_t ringSize = _garbageRing.size();
+                int32_t ringSize = (int32_t)(_garbageRing.size());
                 for (int32_t i = 0; i < ringSize; ++i) {
                     int32_t j = (_ringIndex + ringSize) % ringSize;
                     if (_garbageRing[j] == key) {
@@ -110,7 +110,7 @@ bool ShapeManager::releaseShape(const btCollisionShape* shape) {
 }
 
 void ShapeManager::collectGarbage() {
-    int numShapes = _garbageRing.size();
+    int numShapes = (int32_t)(_garbageRing.size());
     for (int i = 0; i < numShapes; ++i) {
         HashKey key(_garbageRing[i]);
         ShapeReference* shapeRef = _shapeMap.find(key);

From 152edd477c3952aab14d458ef55d466fa7798c75 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 13 Feb 2019 10:00:51 -0800
Subject: [PATCH 7/9] change trace context to rendering

---
 interface/src/ui/ApplicationOverlay.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/interface/src/ui/ApplicationOverlay.cpp b/interface/src/ui/ApplicationOverlay.cpp
index 3579776213..8270b8d307 100644
--- a/interface/src/ui/ApplicationOverlay.cpp
+++ b/interface/src/ui/ApplicationOverlay.cpp
@@ -90,7 +90,7 @@ void ApplicationOverlay::renderOverlay(RenderArgs* renderArgs) {
 }
 
 void ApplicationOverlay::renderQmlUi(RenderArgs* renderArgs) {
-    PROFILE_RANGE(app, __FUNCTION__);
+    PROFILE_RANGE(render, __FUNCTION__);
 
     if (!_uiTexture) {
         _uiTexture = gpu::Texture::createExternal(OffscreenQmlSurface::getDiscardLambda());
@@ -119,7 +119,7 @@ void ApplicationOverlay::renderQmlUi(RenderArgs* renderArgs) {
 }
 
 void ApplicationOverlay::renderOverlays(RenderArgs* renderArgs) {
-    PROFILE_RANGE(app, __FUNCTION__);
+    PROFILE_RANGE(render, __FUNCTION__);
 
     gpu::Batch& batch = *renderArgs->_batch;
     auto geometryCache = DependencyManager::get<GeometryCache>();
@@ -178,7 +178,7 @@ static const auto DEFAULT_SAMPLER = gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_LI
 static const auto DEPTH_FORMAT = gpu::Element(gpu::SCALAR, gpu::FLOAT, gpu::DEPTH);
 
 void ApplicationOverlay::buildFramebufferObject() {
-    PROFILE_RANGE(app, __FUNCTION__);
+    PROFILE_RANGE(render, __FUNCTION__);
 
     auto uiSize = glm::uvec2(qApp->getUiSize());
     if (!_overlayFramebuffer || uiSize != _overlayFramebuffer->getSize()) {

From c79e3e5a6d7eab0c0b5e374ec691c7c812b6d2ad Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 13 Feb 2019 10:02:58 -0800
Subject: [PATCH 8/9] add some trace contexts for easier debugging

---
 interface/src/Application.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index b3c978df9a..615e9f1440 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -6311,7 +6311,10 @@ void Application::update(float deltaTime) {
                 {
                     PROFILE_RANGE(simulation_physics, "RemoveEntities");
                     const VectorOfMotionStates& motionStates = _entitySimulation->getObjectsToRemoveFromPhysics();
-                    _physicsEngine->removeObjects(motionStates);
+                    {
+                        PROFILE_RANGE_EX(simulation_physics, "NumObjs", 0xffff0000, (uint64_t)motionStates.size());
+                        _physicsEngine->removeObjects(motionStates);
+                    }
                     _entitySimulation->deleteObjectsRemovedFromPhysics();
                 }
 
@@ -6320,6 +6323,7 @@ void Application::update(float deltaTime) {
                     VectorOfMotionStates motionStates;
                     getEntities()->getTree()->withReadLock([&] {
                         _entitySimulation->getObjectsToAddToPhysics(motionStates);
+                        PROFILE_RANGE_EX(simulation_physics, "NumObjs", 0xffff0000, (uint64_t)motionStates.size());
                         _physicsEngine->addObjects(motionStates);
                     });
                 }

From ba91bab224a7fdab673bb36f91143a17d007c14f Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 13 Feb 2019 10:05:32 -0800
Subject: [PATCH 9/9] remove cruft

---
 libraries/physics/src/ShapeManager.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libraries/physics/src/ShapeManager.h b/libraries/physics/src/ShapeManager.h
index 7583b7d919..c1fb57e017 100644
--- a/libraries/physics/src/ShapeManager.h
+++ b/libraries/physics/src/ShapeManager.h
@@ -78,8 +78,6 @@ private:
 
     // btHashMap is required because it supports memory alignment of the btCollisionShapes
     btHashMap<HashKey, ShapeReference> _shapeMap;
-    //btAlignedObjectArray<uint64_t> _pendingGarbage;
-
     std::vector<uint64_t> _garbageRing;
     uint32_t _ringIndex { 0 };
 };