Skip to content

Commit

Permalink
18973: Fixes crash when initializing query caches when the first time…
Browse files Browse the repository at this point in the history
… the caches are built is for concurrent operations (#55)
  • Loading branch information
howsohazard authored Jan 13, 2024
1 parent f992b82 commit ec4cf52
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 16 deletions.
4 changes: 1 addition & 3 deletions src/Amalgam/entity/Entity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,14 +792,12 @@ StringInternPool::StringID Entity::GetContainedEntityIdFromIndex(size_t entity_i
return contained_entities[entity_index]->GetIdStringId();
}

EntityQueryCaches *Entity::GetOrCreateQueryCaches()
void Entity::CreateQueryCaches()
{
EnsureHasContainedEntities();

if(!entityRelationships.relationships->queryCaches)
entityRelationships.relationships->queryCaches = std::make_unique<EntityQueryCaches>(this);

return entityRelationships.relationships->queryCaches.get();
}

void Entity::SetRandomState(const std::string &new_state, bool deep_set_seed, std::vector<EntityWriteListener *> *write_listeners)
Expand Down
57 changes: 50 additions & 7 deletions src/Amalgam/entity/Entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,23 @@ class Entity
return entityRelationships.container;
}

//returns true if the entity has one or more contained entities and has a query cache built
bool HasQueryCaches()
{
if(!hasContainedEntities || !entityRelationships.relationships->queryCaches)
return false;
return true;
}

//clears any query caches if they exist
inline void ClearQueryCaches()
{
if(hasContainedEntities)
entityRelationships.relationships->queryCaches.release();
}

//returns a pointer to the query caches for this entity
//creates one if it does not have an active cache
EntityQueryCaches *GetOrCreateQueryCaches();
//creates a cache if it does not exist
void CreateQueryCaches();

//returns a pointer to the query caches for this entity
//returns a nullptr if does not have an active cache
Expand Down Expand Up @@ -864,20 +871,56 @@ class EntityReferenceWithLock : public EntityReferenceBase

//acts as a reference to an Entity that can be treated as an Entity *
// but also performs a read-lock on the container if multithreaded, and frees the read lock when goes out of scope
typedef EntityReferenceWithLock<Concurrency::ReadLock> EntityReadReference;
//can't be a typedef due to the inability to do forward declarations, so have to include constructors
class EntityReadReference : public EntityReferenceWithLock<Concurrency::ReadLock>
{
public:
EntityReadReference() : EntityReferenceWithLock<Concurrency::ReadLock>()
{ }

EntityReadReference(Entity *e) : EntityReferenceWithLock<Concurrency::ReadLock>(e)
{ }
};

//acts as a reference to an Entity that can be treated as an Entity *
// but also performs a write-lock on the container if multithreaded, and frees the read lock when goes out of scope
typedef EntityReferenceWithLock<Concurrency::WriteLock> EntityWriteReference;
//can't be a typedef due to the inability to do forward declarations, so have to include constructors
class EntityWriteReference : public EntityReferenceWithLock<Concurrency::WriteLock>
{
public:
EntityWriteReference() : EntityReferenceWithLock<Concurrency::WriteLock>()
{ }

EntityWriteReference(Entity *e) : EntityReferenceWithLock<Concurrency::WriteLock>(e)
{ }
};

#else //not MULTITHREAD_SUPPORT

//acts as a reference to an Entity that can be treated as an Entity *
// but also performs a read-lock on the container if multithreaded, and frees the read lock when goes out of scope
typedef EntityReferenceBase EntityReadReference;
//can't be a typedef due to the inability to do forward declarations, so have to include constructors
class EntityReadReference : public EntityReferenceBase
{
public:
EntityReadReference() : EntityReferenceBase()
{ }

EntityReadReference(Entity *e) : EntityReferenceBase(e)
{ }
};

//acts as a reference to an Entity that can be treated as an Entity *
// but also performs a write-lock on the container if multithreaded, and frees the read lock when goes out of scope
typedef EntityReferenceBase EntityWriteReference;
//can't be a typedef due to the inability to do forward declarations, so have to include constructors
class EntityWriteReference : public EntityReferenceBase
{
public:
EntityWriteReference() : EntityReferenceBase()
{ }

EntityWriteReference(Entity *e) : EntityReferenceBase(e)
{ }
};

#endif
42 changes: 37 additions & 5 deletions src/Amalgam/entity/EntityQueryCaches.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ EvaluableNodeReference EntityQueryCaches::GetMatchingEntitiesFromQueryCaches(Ent
{
//get the label existance cache associated with this container
// use the first condition as an heuristic for building it if it doesn't exist
EntityQueryCaches *entity_caches = container->GetOrCreateQueryCaches();
EntityQueryCaches *entity_caches = container->GetQueryCaches();

//starting collection of matching entities, initialized to all entities with the requested labels
// reuse existing buffer
Expand Down Expand Up @@ -1299,10 +1299,27 @@ EvaluableNodeReference EntityQueryCaches::GetMatchingEntitiesFromQueryCaches(Ent
}


EvaluableNodeReference EntityQueryCaches::GetEntitiesMatchingQuery(Entity *container, std::vector<EntityQueryCondition> &conditions, EvaluableNodeManager *enm, bool return_query_value)
EvaluableNodeReference EntityQueryCaches::GetEntitiesMatchingQuery(EntityReadReference &container, std::vector<EntityQueryCondition> &conditions, EvaluableNodeManager *enm, bool return_query_value)
{
if(_enable_SBF_datastore && CanUseQueryCaches(conditions))
{
//if haven't built a cache before, need to build the cache container
//need to lock the entity to prevent multiple caches from being built concurrently and overwritten
if(!container->HasQueryCaches())
{
#ifdef MULTITHREAD_SUPPORT
container.lock.unlock();
EntityWriteReference write_lock(container);
container->CreateQueryCaches();
write_lock.lock.unlock();
container.lock.lock();
#else
container->CreateQueryCaches();
#endif
}

return GetMatchingEntitiesFromQueryCaches(container, conditions, enm, return_query_value);
}

if(container == nullptr)
return EvaluableNodeReference(enm->AllocNode(ENT_LIST), true);
Expand All @@ -1324,10 +1341,25 @@ EvaluableNodeReference EntityQueryCaches::GetEntitiesMatchingQuery(Entity *conta
if(conditions[cond_index].queryType == ENT_COMPUTE_ENTITY_CONVICTIONS || conditions[cond_index].queryType == ENT_COMPUTE_ENTITY_KL_DIVERGENCES
|| conditions[cond_index].queryType == ENT_COMPUTE_ENTITY_GROUP_KL_DIVERGENCE || conditions[cond_index].queryType == ENT_COMPUTE_ENTITY_DISTANCE_CONTRIBUTIONS)
{
if(CanUseQueryCaches(conditions))
return GetMatchingEntitiesFromQueryCaches(container, conditions, enm, return_query_value);
else
if(!CanUseQueryCaches(conditions))
return EvaluableNodeReference::Null();

//if haven't built a cache before, need to build the cache container
//need to lock the entity to prevent multiple caches from being built concurrently and overwritten
if(!container->HasQueryCaches())
{
#ifdef MULTITHREAD_SUPPORT
container.lock.unlock();
EntityWriteReference write_lock(container);
container->CreateQueryCaches();
write_lock.lock.unlock();
container.lock.lock();
#else
container->CreateQueryCaches();
#endif
}

return GetMatchingEntitiesFromQueryCaches(container, conditions, enm, return_query_value);
}

query_return_value = conditions[cond_index].GetMatchingEntities(container, matching_entities, first_condition, (return_query_value && last_condition) ? enm : nullptr);
Expand Down
3 changes: 2 additions & 1 deletion src/Amalgam/entity/EntityQueryCaches.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
//forward declarations:
class Entity;
class EntityQueryCondition;
class EntityReadReference;

//stores all of the types of caches needed for queries on a particular entity
class EntityQueryCaches
Expand Down Expand Up @@ -123,7 +124,7 @@ class EntityQueryCaches
//searches container for contained entities matching query.
// if return_query_value is false, then returns a list of all IDs of matching contained entities
// if return_query_value is true, then returns whatever the appropriate structure is for the query type for the final query
static EvaluableNodeReference GetEntitiesMatchingQuery(Entity *container, std::vector<EntityQueryCondition> &conditions, EvaluableNodeManager *enm, bool return_query_value);
static EvaluableNodeReference GetEntitiesMatchingQuery(EntityReadReference &container, std::vector<EntityQueryCondition> &conditions, EvaluableNodeManager *enm, bool return_query_value);

//returns the collection of entities (and optionally associated compute values) that satisfy the specified chain of query conditions
// uses efficient querying methods with a query database, one database per container
Expand Down

0 comments on commit ec4cf52

Please sign in to comment.