Merge pull request #10847 from AndrewMeadows/entitymap

faster EntityItem lookup by EntityItemID
This commit is contained in:
Andrew Meadows 2017-07-18 08:04:37 -07:00 committed by GitHub
commit 5f4df0da2b
9 changed files with 189 additions and 78 deletions

View file

@ -46,10 +46,8 @@ bool AddEntityOperator::preRecursion(const OctreeElementPointer& element) {
// If this element is the best fit for the new entity properties, then add/or update it
if (entityTreeElement->bestFitBounds(_newEntityBox)) {
_tree->addEntityMapEntry(_newEntity);
entityTreeElement->addEntityItem(_newEntity);
_tree->setContainingElement(_newEntity->getEntityItemID(), entityTreeElement);
_foundNew = true;
keepSearching = false;
} else {

View file

@ -96,7 +96,7 @@ bool DeleteEntityOperator::preRecursion(const OctreeElementPointer& element) {
bool entityDeleted = entityTreeElement->removeEntityItem(theEntity); // remove it from the element
assert(entityDeleted);
(void)entityDeleted; // quite warning
_tree->setContainingElement(details.entity->getEntityItemID(), NULL); // update or id to element lookup
_tree->clearEntityMapEntry(details.entity->getEntityItemID());
_foundCount++;
}
}

View file

@ -89,7 +89,8 @@ EntityItem::EntityItem(const EntityItemID& entityItemID) :
EntityItem::~EntityItem() {
// clear out any left-over actions
EntityTreePointer entityTree = _element ? _element->getTree() : nullptr;
EntityTreeElementPointer element = _element; // use local copy of _element for logic below
EntityTreePointer entityTree = element ? element->getTree() : nullptr;
EntitySimulationPointer simulation = entityTree ? entityTree->getSimulation() : nullptr;
if (simulation) {
clearActions(simulation);
@ -880,8 +881,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
// Tracking for editing roundtrips here. We will tell our EntityTree that we just got incoming data about
// and entity that was edited at some time in the past. The tree will determine how it wants to track this
// information.
if (_element && _element->getTree()) {
_element->getTree()->trackIncomingEntityLastEdited(lastEditedFromBufferAdjusted, bytesRead);
EntityTreeElementPointer element = _element; // use local copy of _element for logic below
if (element && element->getTree()) {
element->getTree()->trackIncomingEntityLastEdited(lastEditedFromBufferAdjusted, bytesRead);
}
@ -2056,7 +2058,8 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulationPoi
_previouslyDeletedActions.insert(actionID, usecTimestampNow());
if (_objectActions.contains(actionID)) {
if (!simulation) {
EntityTreePointer entityTree = _element ? _element->getTree() : nullptr;
EntityTreeElementPointer element = _element; // use local copy of _element for logic below
EntityTreePointer entityTree = element ? element->getTree() : nullptr;
simulation = entityTree ? entityTree->getSimulation() : nullptr;
}

View file

@ -90,13 +90,17 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) {
if (_simulation) {
_simulation->clearEntities();
}
{
QWriteLocker locker(&_entityToElementLock);
foreach(EntityTreeElementPointer element, _entityToElementMap) {
element->cleanupEntities();
QHash<EntityItemID, EntityItemPointer> localMap;
localMap.swap(_entityMap);
this->withWriteLock([&] {
foreach(EntityItemPointer entity, localMap) {
EntityTreeElementPointer element = entity->getElement();
if (element) {
element->cleanupEntities();
}
}
_entityToElementMap.clear();
}
});
localMap.clear();
Octree::eraseAllOctreeElements(createNewRoot);
resetClientEditStats();
@ -136,29 +140,24 @@ void EntityTree::postAddEntity(EntityItemPointer entity) {
}
bool EntityTree::updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode) {
EntityTreeElementPointer containingElement = getContainingElement(entityID);
EntityItemPointer entity;
{
QReadLocker locker(&_entityMapLock);
entity = _entityMap.value(entityID);
}
if (!entity) {
return false;
}
return updateEntity(entity, properties, senderNode);
}
bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperties& origProperties,
const SharedNodePointer& senderNode) {
EntityTreeElementPointer containingElement = entity->getElement();
if (!containingElement) {
return false;
}
EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID);
if (!existingEntity) {
return false;
}
return updateEntityWithElement(existingEntity, properties, containingElement, senderNode);
}
bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode) {
EntityTreeElementPointer containingElement = getContainingElement(entity->getEntityItemID());
if (!containingElement) {
return false;
}
return updateEntityWithElement(entity, properties, containingElement, senderNode);
}
bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& origProperties,
EntityTreeElementPointer containingElement, const SharedNodePointer& senderNode) {
EntityItemProperties properties = origProperties;
bool allowLockChange;
@ -190,7 +189,7 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI
bool success;
AACube queryCube = entity->getQueryAACube(success);
if (!success) {
qCDebug(entities) << "Warning -- failed to get query-cube for" << entity->getID();
qCWarning(entities) << "failed to get query-cube for" << entity->getID();
}
UpdateEntityOperator theOperator(getThisPointer(), containingElement, entity, queryCube);
recurseTreeWithOperator(&theOperator);
@ -331,9 +330,8 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI
}
// TODO: this final containingElement check should eventually be removed (or wrapped in an #ifdef DEBUG).
containingElement = getContainingElement(entity->getEntityItemID());
if (!containingElement) {
qCDebug(entities) << "UNEXPECTED!!!! after updateEntity() we no longer have a containing element??? entityID="
if (!entity->getElement()) {
qCWarning(entities) << "EntityTree::updateEntity() we no longer have a containing element for entityID="
<< entity->getEntityItemID();
return false;
}
@ -366,7 +364,7 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti
// You should not call this on existing entities that are already part of the tree! Call updateEntity()
EntityTreeElementPointer containingElement = getContainingElement(entityID);
if (containingElement) {
qCDebug(entities) << "UNEXPECTED!!! ----- don't call addEntity() on existing entity items. entityID=" << entityID
qCWarning(entities) << "EntityTree::addEntity() on existing entity item with entityID=" << entityID
<< "containingElement=" << containingElement.get();
return result;
}
@ -422,7 +420,7 @@ void EntityTree::deleteEntity(const EntityItemID& entityID, bool force, bool ign
EntityTreeElementPointer containingElement = getContainingElement(entityID);
if (!containingElement) {
if (!ignoreWarnings) {
qCDebug(entities) << "UNEXPECTED!!!! EntityTree::deleteEntity() entityID doesn't exist!!! entityID=" << entityID;
qCWarning(entities) << "EntityTree::deleteEntity() on non-existent entityID=" << entityID;
}
return;
}
@ -430,8 +428,7 @@ void EntityTree::deleteEntity(const EntityItemID& entityID, bool force, bool ign
EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID);
if (!existingEntity) {
if (!ignoreWarnings) {
qCDebug(entities) << "UNEXPECTED!!!! don't call EntityTree::deleteEntity() on entity items that don't exist. "
"entityID=" << entityID;
qCWarning(entities) << "EntityTree::deleteEntity() on non-existant entity item with entityID=" << entityID;
}
return;
}
@ -478,7 +475,7 @@ void EntityTree::deleteEntities(QSet<EntityItemID> entityIDs, bool force, bool i
EntityTreeElementPointer containingElement = getContainingElement(entityID);
if (!containingElement) {
if (!ignoreWarnings) {
qCDebug(entities) << "UNEXPECTED!!!! EntityTree::deleteEntities() entityID doesn't exist!!! entityID=" << entityID;
qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entityID=" << entityID;
}
continue;
}
@ -486,8 +483,7 @@ void EntityTree::deleteEntities(QSet<EntityItemID> entityIDs, bool force, bool i
EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID);
if (!existingEntity) {
if (!ignoreWarnings) {
qCDebug(entities) << "UNEXPECTED!!!! don't call EntityTree::deleteEntities() on entity items that don't exist. "
"entityID=" << entityID;
qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entity item with entityID=" << entityID;
}
continue;
}
@ -975,7 +971,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c
const SharedNodePointer& senderNode) {
if (!getIsServer()) {
qCDebug(entities) << "UNEXPECTED!!! processEditPacketData() should only be called on a server tree.";
qCWarning(entities) << "EntityTree::processEditPacketData() should only be called on a server tree.";
return 0;
}
@ -1502,27 +1498,43 @@ int EntityTree::processEraseMessageDetails(const QByteArray& dataByteArray, cons
}
EntityTreeElementPointer EntityTree::getContainingElement(const EntityItemID& entityItemID) /*const*/ {
QReadLocker locker(&_entityToElementLock);
EntityTreeElementPointer element = _entityToElementMap.value(entityItemID);
return element;
EntityItemPointer entity;
{
QReadLocker locker(&_entityMapLock);
entity = _entityMap.value(entityItemID);
}
if (entity) {
return entity->getElement();
}
return EntityTreeElementPointer(nullptr);
}
void EntityTree::setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element) {
QWriteLocker locker(&_entityToElementLock);
if (element) {
_entityToElementMap[entityItemID] = element;
} else {
_entityToElementMap.remove(entityItemID);
void EntityTree::addEntityMapEntry(EntityItemPointer entity) {
EntityItemID id = entity->getEntityItemID();
QWriteLocker locker(&_entityMapLock);
EntityItemPointer otherEntity = _entityMap.value(id);
if (otherEntity) {
qCWarning(entities) << "EntityTree::addEntityMapEntry() found pre-existing id " << id;
assert(false);
return;
}
_entityMap.insert(id, entity);
}
void EntityTree::clearEntityMapEntry(const EntityItemID& id) {
QWriteLocker locker(&_entityMapLock);
_entityMap.remove(id);
}
void EntityTree::debugDumpMap() {
// QHash's are implicitly shared, so we make a shared copy and use that instead.
// This way we might be able to avoid both a lock and a true copy.
QHash<EntityItemID, EntityItemPointer> localMap(_entityMap);
qCDebug(entities) << "EntityTree::debugDumpMap() --------------------------";
QReadLocker locker(&_entityToElementLock);
QHashIterator<EntityItemID, EntityTreeElementPointer> i(_entityToElementMap);
QHashIterator<EntityItemID, EntityItemPointer> i(localMap);
while (i.hasNext()) {
i.next();
qCDebug(entities) << i.key() << ": " << i.value().get();
qCDebug(entities) << i.key() << ": " << i.value()->getElement().get();
}
qCDebug(entities) << "-----------------------------------------------------";
}

View file

@ -119,9 +119,6 @@ public:
// use this method if you only know the entityID
bool updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr));
// use this method if you have a pointer to the entity (avoid an extra entity lookup)
bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr));
// check if the avatar is a child of this entity, If so set the avatar parentID to null
void unhookChildAvatar(const EntityItemID entityID);
void deleteEntity(const EntityItemID& entityID, bool force = false, bool ignoreWarnings = true);
@ -183,7 +180,8 @@ public:
int processEraseMessageDetails(const QByteArray& buffer, const SharedNodePointer& sourceNode);
EntityTreeElementPointer getContainingElement(const EntityItemID& entityItemID) /*const*/;
void setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element);
void addEntityMapEntry(EntityItemPointer entity);
void clearEntityMapEntry(const EntityItemID& id);
void debugDumpMap();
virtual void dumpTree() override;
virtual void pruneTree() override;
@ -275,9 +273,8 @@ signals:
protected:
void processRemovedEntities(const DeleteEntityOperator& theOperator);
bool updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& properties,
EntityTreeElementPointer containingElement,
const SharedNodePointer& senderNode = SharedNodePointer(nullptr));
bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties,
const SharedNodePointer& senderNode = SharedNodePointer(nullptr));
static bool findNearPointOperation(const OctreeElementPointer& element, void* extraData);
static bool findInSphereOperation(const OctreeElementPointer& element, void* extraData);
static bool findInCubeOperation(const OctreeElementPointer& element, void* extraData);
@ -309,8 +306,8 @@ protected:
_deletedEntityItemIDs << id;
}
mutable QReadWriteLock _entityToElementLock;
QHash<EntityItemID, EntityTreeElementPointer> _entityToElementMap;
mutable QReadWriteLock _entityMapLock;
QHash<EntityItemID, EntityItemPointer> _entityMap;
EntitySimulationPointer _simulation;

View file

@ -885,10 +885,10 @@ EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemI
void EntityTreeElement::cleanupEntities() {
withWriteLock([&] {
foreach(EntityItemPointer entity, _entityItems) {
// NOTE: only EntityTreeElement should ever be changing the value of entity->_element
// NOTE: We explicitly don't delete the EntityItem here because since we only
// access it by smart pointers, when we remove it from the _entityItems
// we know that it will be deleted.
//delete entity;
entity->_element = NULL;
}
_entityItems.clear();
@ -903,6 +903,7 @@ bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) {
EntityItemPointer& entity = _entityItems[i];
if (entity->getEntityItemID() == id) {
foundEntity = true;
// NOTE: only EntityTreeElement should ever be changing the value of entity->_element
entity->_element = NULL;
_entityItems.removeAt(i);
break;
@ -918,6 +919,7 @@ bool EntityTreeElement::removeEntityItem(EntityItemPointer entity) {
numEntries = _entityItems.removeAll(entity);
});
if (numEntries > 0) {
// NOTE: only EntityTreeElement should ever be changing the value of entity->_element
assert(entity->_element.get() == this);
entity->_element = NULL;
return true;
@ -1001,7 +1003,6 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int
if (currentContainingElement.get() != this) {
currentContainingElement->removeEntityItem(entityItem);
addEntityItem(entityItem);
_myTree->setContainingElement(entityItemID, getThisPointer());
}
}
}
@ -1032,9 +1033,9 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int
// don't add if we've recently deleted....
if (!_myTree->isDeletedEntity(entityItem->getID())) {
_myTree->addEntityMapEntry(entityItem);
addEntityItem(entityItem); // add this new entity to this elements entities
entityItemID = entityItem->getEntityItemID();
_myTree->setContainingElement(entityItemID, getThisPointer());
_myTree->postAddEntity(entityItem);
if (entityItem->getCreated() == UNKNOWN_CREATED_TIME) {
entityItem->recordCreationTime();

View file

@ -51,7 +51,7 @@ MovingEntitiesOperator::~MovingEntitiesOperator() {
void MovingEntitiesOperator::addEntityToMoveList(EntityItemPointer entity, const AACube& newCube) {
EntityTreeElementPointer oldContainingElement = _tree->getContainingElement(entity->getEntityItemID());
EntityTreeElementPointer oldContainingElement = entity->getElement();
AABox newCubeClamped = newCube.clamp((float)-HALF_TREE_SCALE, (float)HALF_TREE_SCALE);
if (_wantDebug) {
@ -193,7 +193,6 @@ bool MovingEntitiesOperator::preRecursion(const OctreeElementPointer& element) {
// If this element is the best fit for the new bounds of this entity then add the entity to the element
if (!details.newFound && entityTreeElement->bestFitBounds(details.newCube)) {
EntityItemID entityItemID = details.entity->getEntityItemID();
// remove from the old before adding
EntityTreeElementPointer oldElement = details.entity->getElement();
if (oldElement != entityTreeElement) {
@ -201,7 +200,6 @@ bool MovingEntitiesOperator::preRecursion(const OctreeElementPointer& element) {
oldElement->removeEntityItem(details.entity);
}
entityTreeElement->addEntityItem(details.entity);
_tree->setContainingElement(entityItemID, entityTreeElement);
}
_foundNewCount++;
//details.newFound = true; // TODO: would be nice to add this optimization

View file

@ -138,8 +138,8 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) {
qCDebug(entities) << " _foundNew=" << _foundNew;
}
// If we haven't yet found the old entity, and this subTreeContains our old
// entity, then we need to keep searching.
// If we haven't yet found the old element, and this subTreeContains our old element,
// then we need to keep searching.
if (!_foundOld && subtreeContainsOld) {
if (_wantDebug) {
@ -169,7 +169,6 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) {
// NOTE: we know we haven't yet added it to its new element because _removeOld is true
EntityTreeElementPointer oldElement = _existingEntity->getElement();
oldElement->removeEntityItem(_existingEntity);
_tree->setContainingElement(_entityItemID, NULL);
if (oldElement != _containingElement) {
qCDebug(entities) << "WARNING entity moved during UpdateEntityOperator recursion";
@ -187,8 +186,8 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) {
}
}
// If we haven't yet found the new entity, and this subTreeContains our new
// entity, then we need to keep searching.
// If we haven't yet found the new element, and this subTreeContains our new element,
// then we need to keep searching.
if (!_foundNew && subtreeContainsNew) {
if (_wantDebug) {
@ -221,7 +220,6 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) {
}
}
entityTreeElement->addEntityItem(_existingEntity);
_tree->setContainingElement(_entityItemID, entityTreeElement);
}
_foundNew = true; // we found the new element
_removeOld = false; // and it has already been removed from the old

View file

@ -0,0 +1,104 @@
// Creates a large number of entities on the cardinal planes of the octree (all
// objects will live in the root octree element). Measures how long it takes
// to update the properties of the first and last entity. The difference
// between the two measurements shows how the cost of lookup changes as a
// function of the number of entities. For best results run this in an
// otherwise empty domain.
var firstId;
var lastId;
var NUM_ENTITIES_ON_SIDE = 25;
// create the objects
createObjects = function () {
var STRIDE = 0.75;
var WIDTH = 0.5;
var DIMENSIONS = { x: WIDTH, y: WIDTH, z: WIDTH };
var LIFETIME = 20;
var properties = {
name: "",
type : "Box",
dimensions : DIMENSIONS,
position : { x: 0, y: 0, z: 0},
lifetime : LIFETIME,
color : { red:255, green: 64, blue: 255 }
};
// xy
var planeName = "xy";
for (var i = 0; i < NUM_ENTITIES_ON_SIDE; ++i) {
for (var j = 0; j < NUM_ENTITIES_ON_SIDE; ++j) {
properties.name = "Box-" + planeName + "-" + i + "." + j;
properties.position = { x: i * STRIDE, y: j * STRIDE, z: 0 };
var red = i * 255 / NUM_ENTITIES_ON_SIDE;
var green = j * 255 / NUM_ENTITIES_ON_SIDE;
var blue = 0;
properties.color = { red: red, green: green, blue: blue };
if (i == 0 && j == 0) {
firstId = Entities.addEntity(properties);
} else {
Entities.addEntity(properties);
}
}
}
// yz
var planeName = "yz";
for (var i = 0; i < NUM_ENTITIES_ON_SIDE; ++i) {
for (var j = 0; j < NUM_ENTITIES_ON_SIDE; ++j) {
properties.name = "Box-" + planeName + "-" + i + "." + j;
properties.position = { x: 0, y: i * STRIDE, z: j * STRIDE };
var red = 0;
var green = i * 255 / NUM_ENTITIES_ON_SIDE;
var blue = j * 255 / NUM_ENTITIES_ON_SIDE;
properties.color = { red: red, green: green, blue: blue };
Entities.addEntity(properties);
}
}
// zx
var planeName = "zx";
for (var i = 0; i < NUM_ENTITIES_ON_SIDE; ++i) {
for (var j = 0; j < NUM_ENTITIES_ON_SIDE; ++j) {
properties.name = "Box-" + planeName + "-" + i + "." + j;
properties.position = { x: j * STRIDE, y: 0, z: i * STRIDE };
var red = j * 255 / NUM_ENTITIES_ON_SIDE;
var green = 0;
var blue = i * 255 / NUM_ENTITIES_ON_SIDE;
properties.color = { red: red, green: green, blue: blue };
lastId = Entities.addEntity(properties);
}
}
};
createObjects();
// measure the time it takes to edit the first and last entities many times
// (requires a lookup by entityId each time)
changeProperties = function (id) {
var newProperties = { color : { red: 255, green: 255, blue: 255 } };
Entities.editEntity(id, newProperties);
}
// first
var NUM_CHANGES = 10000;
var firstStart = Date.now();
for (var k = 0; k < NUM_CHANGES; ++k) {
changeProperties(firstId);
}
var firstEnd = Date.now();
var firstDt = firstEnd - firstStart;
// last
var lastStart = Date.now();
for (var k = 0; k < NUM_CHANGES; ++k) {
changeProperties(lastId);
}
var lastEnd = Date.now();
var lastDt = lastEnd - lastStart;
// print the results
var numEntities = 3 * NUM_ENTITIES_ON_SIDE * NUM_ENTITIES_ON_SIDE;
print("numEntities = " + numEntities + " numEdits = " + NUM_CHANGES + " firstDt = " + firstDt + " lastDt = " + lastDt);