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
minetest · Apr 16, 2020 · e8ac5a3 · e8ac5a3 · oilboi · Apr 17, 2020
1 parent 62ae7ad
commit e8ac5a3
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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
@@ -35,8 +35,9 @@ class ActiveObjectMgr : public ::ActiveObjectMgr<ServerActiveObject>
 	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
@@ -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
@@ -323,9 +323,10 @@ class ServerEnvironment : public Environment
 	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
@@ -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);
 }