Add minetest.bulk_set_node call + optimize Environment::set_node call (…

…#6958) * Add minetest.bulk_set_node call + experimental mod unittest * Optimize set_node function to prevent triple lookup on contentfeatures Do only one lookup for old, and try to merge old and new lookup if node is same than previous node * Add benchmark function + optimize vector population to have real results
minetest · Jan 29, 2018 · 584d00a · HybridDog · Feb 2, 2018 · 584d00a
1 parent 3b4df95
commit 584d00a
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 3 deletions.
diff --git a/doc/lua_api.txt b/doc/lua_api.txt
@@ -2749,6 +2749,15 @@ and `minetest.auth_reload` call the authentication handler.
     * `node`: table `{name=string, param1=number, param2=number}`
     * If param1 or param2 is omitted, it's set to `0`.
     * e.g. `minetest.set_node({x=0, y=10, z=0}, {name="default:wood"})`
+* `minetest.bulk_set_node({pos1, pos2, pos3, ...}, node)`
+    * Set node on all positions set in the first argument.
+    * e.g. `minetest.bulk_set_node({{x=0, y=1, z=1}, {x=1, y=2, z=2}}, {name="default:stone"})`
+    * For node specification or position syntax see `minetest.set_node` call
+    * Faster than set_node due to single call, but still considerably slower than 
+      Voxel Manipulators (LVM) for large numbers of nodes.
+      Unlike LVMs, this will call node callbacks. It also allows setting nodes in spread out 
+      positions which would cause LVMs to waste memory.
+      For setting a cube, this is 1.3x faster than set_node whereas LVM is 20x faster.
 * `minetest.swap_node(pos, node)`
     * Set node at position, but don't remove metadata
 * `minetest.remove_node(pos)`

diff --git a/games/minimal/mods/experimental/init.lua b/games/minimal/mods/experimental/init.lua
@@ -682,6 +682,74 @@ minetest.register_chatcommand("test1", {
 	end,
 })
 
+minetest.register_chatcommand("test_bulk_set_node", {
+	params = "",
+	description = "Test 2: bulk set a node",
+	func = function(name, param)
+		local player = minetest.get_player_by_name(name)
+		if not player then
+			return
+		end
+		local pos_list = {}
+		local ppos = player:get_pos()
+		local i = 1
+		for x=2,10 do
+			for y=2,10 do
+				for z=2,10 do
+					pos_list[i] = {x=ppos.x + x,y = ppos.y + y,z = ppos.z + z}
+					i = i + 1
+				end
+			end
+		end
+		minetest.bulk_set_node(pos_list, {name = "default:stone"})
+		minetest.chat_send_player(name, "Done.");
+	end,
+})
+
+minetest.register_chatcommand("bench_bulk_set_node", {
+	params = "",
+	description = "Test 3: bulk set a node (bench)",
+	func = function(name, param)
+		local player = minetest.get_player_by_name(name)
+		if not player then
+			return
+		end
+		local pos_list = {}
+		local ppos = player:get_pos()
+		local i = 1
+		for x=2,100 do
+			for y=2,100 do
+				for z=2,100 do
+					pos_list[i] = {x=ppos.x + x,y = ppos.y + y,z = ppos.z + z}
+					i = i + 1
+				end
+			end
+		end
+
+		minetest.chat_send_player(name, "Benching bulk set node. Warming up...");
+
+		-- warm up with default:stone to prevent having different callbacks
+		-- due to different node topology
+		minetest.bulk_set_node(pos_list, {name = "default:stone"})
+
+		minetest.chat_send_player(name, "Warming up finished, now benching...");
+
+		local start_time = os.clock()
+		for i=1,#pos_list do
+			minetest.set_node(pos_list[i], {name = "default:stone"})
+		end
+		local middle_time = os.clock()
+		minetest.bulk_set_node(pos_list, {name = "default:stone"})
+		local end_time = os.clock()
+		minetest.chat_send_player(name,
+			string.format("Bench results: set_node loop[%.2fms], bulk_set_node[%.2fms]",
+				(middle_time - start_time) * 1000,
+				(end_time - middle_time) * 1000
+			)
+		);
+	end,
+})
+
 minetest.register_on_player_receive_fields(function(player, formname, fields)
 	experimental.print_to_everything("Inventory fields 1: player="..player:get_player_name()..", fields="..dump(fields))
 end)

diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp
@@ -273,6 +273,39 @@ int ModApiEnvMod::l_set_node(lua_State *L)
 	return 1;
 }
 
+// bulk_set_node([pos1, pos2, ...], node)
+// pos = {x=num, y=num, z=num}
+int ModApiEnvMod::l_bulk_set_node(lua_State *L)
+{
+	GET_ENV_PTR;
+
+	INodeDefManager *ndef = env->getGameDef()->ndef();
+	// parameters
+	if (!lua_istable(L, 1)) {
+		return 0;
+	}
+
+	s32 len = lua_objlen(L, 1);
+	if (len == 0) {
+		lua_pushboolean(L, true);
+		return 1;
+	}
+
+	MapNode n = readnode(L, 2, ndef);
+
+	// Do it
+	bool succeeded = true;
+	for (s32 i = 1; i <= len; i++) {
+		lua_rawgeti(L, 1, i);
+		if (!env->setNode(read_v3s16(L, -1), n))
+			succeeded = false;
+		lua_pop(L, 1);
+	}
+
+	lua_pushboolean(L, succeeded);
+	return 1;
+}
+
 int ModApiEnvMod::l_add_node(lua_State *L)
 {
 	return l_set_node(L);
@@ -1232,6 +1265,7 @@ int ModApiEnvMod::l_forceload_free_block(lua_State *L)
 void ModApiEnvMod::Initialize(lua_State *L, int top)
 {
 	API_FCT(set_node);
+	API_FCT(bulk_set_node);
 	API_FCT(add_node);
 	API_FCT(swap_node);
 	API_FCT(add_item);

diff --git a/src/script/lua_api/l_env.h b/src/script/lua_api/l_env.h
@@ -29,6 +29,10 @@ class ModApiEnvMod : public ModApiBase {
 	// pos = {x=num, y=num, z=num}
 	static int l_set_node(lua_State *L);
 
+	// bulk_set_node([pos1, pos2, ...], node)
+	// pos = {x=num, y=num, z=num}
+	static int l_bulk_set_node(lua_State *L);
+
 	static int l_add_node(lua_State *L);
 
 	// remove_node(pos)

diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
@@ -917,8 +917,10 @@ bool ServerEnvironment::setNode(v3s16 p, const MapNode &n)
 	INodeDefManager *ndef = m_server->ndef();
 	MapNode n_old = m_map->getNodeNoEx(p);
 
+	const ContentFeatures &cf_old = ndef->get(n_old);
+
 	// Call destructor
-	if (ndef->get(n_old).has_on_destruct)
+	if (cf_old.has_on_destruct)
 		m_script->node_on_destruct(p, n_old);
 
 	// Replace node
@@ -929,11 +931,15 @@ bool ServerEnvironment::setNode(v3s16 p, const MapNode &n)
 	m_map->updateVManip(p);
 
 	// Call post-destructor
-	if (ndef->get(n_old).has_after_destruct)
+	if (cf_old.has_after_destruct)
 		m_script->node_after_destruct(p, n_old);
 
+	// Retrieve node content features
+	// if new node is same as old, reuse old definition to prevent a lookup
+	const ContentFeatures &cf_new = n_old == n ? cf_old : ndef->get(n);
+
 	// Call constructor
-	if (ndef->get(n).has_on_construct)
+	if (cf_new.has_on_construct)
 		m_script->node_on_construct(p, n);
 
 	return true;