Optimize get_objects_inside_radius calls (#9671)

* Optimize getObjectsInsideRadius calls

our previous implementation calls the ActiveObjectMgr to return ids and then lookup those ids in the same map and test each object
Instead now we call the global map to return the pointers directly and we ask filtering when building the list using lamba.

This drop double looping over ranges of active objects (and then filtered one) and drop x lookups on the map regarding the first call results

diff --git a/src/collision.cpp b/src/collision.cpp
index d9fbd3202419cb7fb598abf441aeb7fa32b5543a..6d24bc699f10fe19acf6d7411b9cedb381a63dad 100644 (file)
--- a/src/collision.cpp
+++ b/src/collision.cpp
@@ -360,17 +360,19 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
                                // Calculate distance by speed, add own extent and 1.5m of tolerance
                                f32 distance = speed_f->getLength() * dtime +
                                        box_0.getExtent().getLength() + 1.5f * BS;
-                               std::vector<u16> s_objects;
-                               s_env->getObjectsInsideRadius(s_objects, *pos_f, distance);
 
-                               for (u16 obj_id : s_objects) {
-                                       ServerActiveObject *current = s_env->getActiveObject(obj_id);
-
-                                       if (!self || (self != current &&
-                                                       self != current->getParent())) {
-                                               objects.push_back((ActiveObject*)current);
+                               // search for objects which are not us, or we are not its parent
+                               // we directly use the callback to populate the result to prevent
+                               // a useless result loop here
+                               auto include_obj_cb = [self, &objects] (ServerActiveObject *obj) {
+                                       if (!self || (self != obj && self != obj->getParent())) {
+                                               objects.push_back((ActiveObject *)obj);
                                        }
-                               }
+                                       return false;
+                               };
+
+                               std::vector<ServerActiveObject *> s_objects;
+                               s_env->getObjectsInsideRadius(s_objects, *pos_f, distance, include_obj_cb);
                        }
                }
 

diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp
index 8c45a1510e15fde1a7aee5feef02398ae0d7539f..831464d3bf77c459c2188e669da1c1f1bd1691d1 100644 (file)
--- a/src/script/lua_api/l_env.cpp
+++ b/src/script/lua_api/l_env.cpp
@@ -681,22 +681,22 @@ int ModApiEnvMod::l_get_player_by_name(lua_State *L)
 int ModApiEnvMod::l_get_objects_inside_radius(lua_State *L)
 {
        GET_ENV_PTR;
+       ScriptApiBase *script = getScriptApiBase(L);
 
        // Do it
        v3f pos = checkFloatPos(L, 1);
        float radius = readParam<float>(L, 2) * BS;
-       std::vector<u16> ids;
-       env->getObjectsInsideRadius(ids, pos, radius);
-       ScriptApiBase *script = getScriptApiBase(L);
-       lua_createtable(L, ids.size(), 0);
-       std::vector<u16>::const_iterator iter = ids.begin();
-       for(u32 i = 0; iter != ids.end(); ++iter) {
-               ServerActiveObject *obj = env->getActiveObject(*iter);
-               if (!obj->isGone()) {
-                       // Insert object reference into table
-                       script->objectrefGetOrCreate(L, obj);
-                       lua_rawseti(L, -2, ++i);
-               }
+       std::vector<ServerActiveObject *> objs;
+
+       auto include_obj_cb = [](ServerActiveObject *obj){ return !obj->isGone(); };
+       env->getObjectsInsideRadius(objs, pos, radius, include_obj_cb);
+
+       int i = 0;
+       lua_createtable(L, objs.size(), 0);
+       for (const auto obj : objs) {
+               // Insert object reference into table
+               script->objectrefGetOrCreate(L, obj);
+               lua_rawseti(L, -2, ++i);
        }
        return 1;
 }

diff --git a/src/server/activeobjectmgr.cpp b/src/server/activeobjectmgr.cpp
index 984ae77941715ed77541a00f29384286e1cb6315..1b8e31409fb8389d3ce6a0bfad36e6b21dc0a403 100644 (file)
--- a/src/server/activeobjectmgr.cpp
+++ b/src/server/activeobjectmgr.cpp
@@ -111,17 +111,19 @@ void ActiveObjectMgr::removeObject(u16 id)
 }
 
 // clang-format on
-void ActiveObjectMgr::getObjectsInsideRadius(
-               const v3f &pos, float radius, std::vector<u16> &result)
+void ActiveObjectMgr::getObjectsInsideRadius(const v3f &pos, float radius,
+               std::vector<ServerActiveObject *> &result,
+               std::function<bool(ServerActiveObject *obj)> include_obj_cb)
 {
        float r2 = radius * radius;
        for (auto &activeObject : m_active_objects) {
                ServerActiveObject *obj = activeObject.second;
-               u16 id = activeObject.first;
                const v3f &objectpos = obj->getBasePosition();
                if (objectpos.getDistanceFromSQ(pos) > r2)
                        continue;
-               result.push_back(id);
+
+               if (!include_obj_cb || include_obj_cb(obj))
+                       result.push_back(obj);
        }
 }
 

diff --git a/src/server/activeobjectmgr.h b/src/server/activeobjectmgr.h
index 5fea1bea67623fdd71c2190d5609a37375e90f99..bc208549929446fc7789cbab0d6c986465de251a 100644 (file)
--- a/src/server/activeobjectmgr.h
+++ b/src/server/activeobjectmgr.h
@@ -35,8 +35,9 @@ public:
        bool registerObject(ServerActiveObject *obj) override;
        void removeObject(u16 id) override;
 
-       void getObjectsInsideRadius(
-                       const v3f &pos, float radius, std::vector<u16> &result);
+       void getObjectsInsideRadius(const v3f &pos, float radius,
+                       std::vector<ServerActiveObject *> &result,
+                       std::function<bool(ServerActiveObject *obj)> include_obj_cb);
 
        void getAddedActiveObjectsAroundPos(const v3f &player_pos, f32 radius,
                        f32 player_radius, std::set<u16> &current_objects,

diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
index 7393846735026cba44de986c3b84fe88ea546326..27f0c1e3d0195253d2ee778210418edb620b5163 100644 (file)
--- a/src/serverenvironment.cpp
+++ b/src/serverenvironment.cpp
@@ -1608,14 +1608,12 @@ void ServerEnvironment::getSelectedActiveObjects(
        const core::line3d<f32> &shootline_on_map,
        std::vector<PointedThing> &objects)
 {
-       std::vector<u16> objectIds;
-       getObjectsInsideRadius(objectIds, shootline_on_map.start,
-               shootline_on_map.getLength() + 10.0f);
+       std::vector<ServerActiveObject *> objs;
+       getObjectsInsideRadius(objs, shootline_on_map.start,
+               shootline_on_map.getLength() + 10.0f, nullptr);
        const v3f line_vector = shootline_on_map.getVector();
 
-       for (u16 objectId : objectIds) {
-               ServerActiveObject* obj = getActiveObject(objectId);
-
+       for (auto obj : objs) {
                aabb3f selection_box;
                if (!obj->getSelectionBox(&selection_box))
                        continue;
@@ -1630,7 +1628,7 @@ void ServerEnvironment::getSelectedActiveObjects(
                if (boxLineCollision(offsetted_box, shootline_on_map.start, line_vector,
                                &current_intersection, &current_normal)) {
                        objects.emplace_back(
-                               (s16) objectId, current_intersection, current_normal,
+                               (s16) obj->getId(), current_intersection, current_normal,
                                (current_intersection - shootline_on_map.start).getLengthSQ());
                }
        }

diff --git a/src/serverenvironment.h b/src/serverenvironment.h
index 55ecbd05f004d41d9f8970724d57d83387d5ee96..f814b95c0297bdadba410dc40dc889f2141e5956 100644 (file)
--- a/src/serverenvironment.h
+++ b/src/serverenvironment.h
@@ -323,9 +323,10 @@ public:
        bool swapNode(v3s16 p, const MapNode &n);
 
        // Find all active objects inside a radius around a point
-       void getObjectsInsideRadius(std::vector<u16> &objects, const v3f &pos, float radius)
+       void getObjectsInsideRadius(std::vector<ServerActiveObject *> &objects, const v3f &pos, float radius,
+                       std::function<bool(ServerActiveObject *obj)> include_obj_cb)
        {
-               return m_ao_manager.getObjectsInsideRadius(pos, radius, objects);
+               return m_ao_manager.getObjectsInsideRadius(pos, radius, objects, include_obj_cb);
        }
 
        // Clear objects, loading and going through every MapBlock

diff --git a/src/unittest/test_serveractiveobjectmgr.cpp b/src/unittest/test_serveractiveobjectmgr.cpp
index 0806972abbbe0ef17ac20f00348ac3b818827754..aa00474003b9ebeb27216ff1689410bc272df26c 100644 (file)
--- a/src/unittest/test_serveractiveobjectmgr.cpp
+++ b/src/unittest/test_serveractiveobjectmgr.cpp
@@ -148,14 +148,26 @@ void TestServerActiveObjectMgr::testGetObjectsInsideRadius()
                saomgr.registerObject(new TestServerActiveObject(p));
        }
 
-       std::vector<u16> result;
-       saomgr.getObjectsInsideRadius(v3f(), 50, result);
+       std::vector<ServerActiveObject *> result;
+       saomgr.getObjectsInsideRadius(v3f(), 50, result, nullptr);
        UASSERTCMP(int, ==, result.size(), 1);
 
        result.clear();
-       saomgr.getObjectsInsideRadius(v3f(), 750, result);
+       saomgr.getObjectsInsideRadius(v3f(), 750, result, nullptr);
        UASSERTCMP(int, ==, result.size(), 2);
 
+       result.clear();
+       saomgr.getObjectsInsideRadius(v3f(), 750000, result, nullptr);
+       UASSERTCMP(int, ==, result.size(), 5);
+
+       result.clear();
+       auto include_obj_cb = [](ServerActiveObject *obj) {
+               return (obj->getBasePosition().X != 10);
+       };
+
+       saomgr.getObjectsInsideRadius(v3f(), 750000, result, include_obj_cb);
+       UASSERTCMP(int, ==, result.size(), 4);
+
        clearSAOMgr(&saomgr);
 }