From e8ac5a31cf12afcfddf8e3ed31e8038930edb06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Blot?= Date: Thu, 16 Apr 2020 08:25:48 +0200 Subject: [PATCH] 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 --- src/collision.cpp | 20 +++++++++-------- src/script/lua_api/l_env.cpp | 24 ++++++++++----------- src/server/activeobjectmgr.cpp | 10 +++++---- src/server/activeobjectmgr.h | 5 +++-- src/serverenvironment.cpp | 12 +++++------ src/serverenvironment.h | 5 +++-- src/unittest/test_serveractiveobjectmgr.cpp | 18 +++++++++++++--- 7 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/collision.cpp b/src/collision.cpp index d9fbd3202..6d24bc699 100644 --- 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 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 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 8c45a1510..831464d3b 100644 --- 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(L, 2) * BS; - std::vector ids; - env->getObjectsInsideRadius(ids, pos, radius); - ScriptApiBase *script = getScriptApiBase(L); - lua_createtable(L, ids.size(), 0); - std::vector::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 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 984ae7794..1b8e31409 100644 --- 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 &result) +void ActiveObjectMgr::getObjectsInsideRadius(const v3f &pos, float radius, + std::vector &result, + std::function 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 5fea1bea6..bc2085499 100644 --- 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 &result); + void getObjectsInsideRadius(const v3f &pos, float radius, + std::vector &result, + std::function include_obj_cb); void getAddedActiveObjectsAroundPos(const v3f &player_pos, f32 radius, f32 player_radius, std::set ¤t_objects, diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index 739384673..27f0c1e3d 100644 --- a/src/serverenvironment.cpp +++ b/src/serverenvironment.cpp @@ -1608,14 +1608,12 @@ void ServerEnvironment::getSelectedActiveObjects( const core::line3d &shootline_on_map, std::vector &objects) { - std::vector objectIds; - getObjectsInsideRadius(objectIds, shootline_on_map.start, - shootline_on_map.getLength() + 10.0f); + std::vector 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, ¤t_intersection, ¤t_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 55ecbd05f..f814b95c0 100644 --- 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 &objects, const v3f &pos, float radius) + void getObjectsInsideRadius(std::vector &objects, const v3f &pos, float radius, + std::function 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 0806972ab..aa0047400 100644 --- 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 result; - saomgr.getObjectsInsideRadius(v3f(), 50, result); + std::vector 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); }