Change ipairs() back to pairs() #1311

New Issue

kay27 · 2021-03-16T12:54:05+01:00

kay27 commented

2021-03-16 12:54:05 +01:00

@AFCMS I've written a test to check your latest optimization (see below).
It seems for me that ipairs() works at least 22 times slower than pairs().
So please please undo latest changes ASAP.
Maybe it's one of the reasons of MineClone2/MineClone2#1283 as well.
I've already mentioned it here MineClone2/MineClone2#1146 (comment)

I would suggest better open new issues before doing any mass optimizations because it affects all the code and should be planned because of that.

(see code block below)

@AFCMS I've written a test to check your latest optimization (see below). It seems for me that ```ipairs()``` works at least 22 times slower than ```pairs()```. So please please undo latest changes ASAP. Maybe it's one of the reasons of https://git.minetest.land/MineClone2/MineClone2/issues/1283 as well. I've already mentioned it here https://git.minetest.land/MineClone2/MineClone2/issues/1146#issuecomment-14856 I would suggest better open new issues before doing any mass optimizations because it affects all the code and should be planned because of that. ![изображение](/attachments/a4d77691-c7bc-4837-9ed1-fb810401f0bc) (see [code block below](https://git.minetest.land/MineClone2/MineClone2/issues/1311#issuecomment-16913))

изображение.png

253 KiB

👍 1 😕 1

kay27 added the

#P1 CRITICAL

performance

labels 2021-03-16 12:54:05 +01:00

AFCMS was assigned by kay27

2021-03-16 13:01:26 +01:00

ryvnf commented

2021-03-16 13:16:30 +01:00

Good you took the time to measure this :)

It makes sense for pairs to be faster because it is a less strict version of ipairs. It is for example valid for pairs to be eqvivalent to ipairs when using only numeric keys and that is most efficient (indices are stored in numerical order). The reverse does not hold true because ipairs needs to iterate over the keys in a deterministic order.

I also think you put the CRITICAL label when you meant to put high priority. Among the labels CRITICAL is described as "for crashes, catastrophic bugs" while high priority is described as "for the most important issues".

Good you took the time to measure this :) It makes sense for `pairs` to be faster because it is a less strict version of `ipairs`. It is for example valid for `pairs` to be eqvivalent to `ipairs` when using only numeric keys and that is most efficient (indices are stored in numerical order). The reverse does not hold true because `ipairs` needs to iterate over the keys in a deterministic order. I also think you put the *CRITICAL* label when you meant to put *high priority*. Among the labels *CRITICAL* is described as "for crashes, catastrophic bugs" while *high priority* is described as "for the most important issues".

AFCMS commented

2021-03-16 13:25:33 +01:00

@kay27 ??????????????????
I don't understand.

minetest.register_on_joinplayer(function(player) 
	local ttt = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"}
	local out = {}
	local time = minetest.get_us_time()
	for i = 1,20000 do
		for k, v in pairs(ttt) do print(v) end
	end
	minetest.chat_send_all("Pairs:  "..tonumber(minetest.get_us_time()-time))
	local time = minetest.get_us_time()
	for i = 1,20000 do
		for k, v in ipairs(ttt) do print(v) end
	end
	minetest.chat_send_all("Ipairs:  "..tonumber(minetest.get_us_time()-time))
	local time = minetest.get_us_time()
	for i = 1,20000 do
		for i = 1, #ttt do print(ttt[i]) end
	end
	minetest.chat_send_all("#:      "..tonumber(minetest.get_us_time()-time))
end)

And I got:
Pairs: 137000
Ipairs: 57000
for i =.... : 75000

@kay27 ?????????????????? I don't understand. ``` minetest.register_on_joinplayer(function(player) local ttt = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"} local out = {} local time = minetest.get_us_time() for i = 1,20000 do for k, v in pairs(ttt) do print(v) end end minetest.chat_send_all("Pairs: "..tonumber(minetest.get_us_time()-time)) local time = minetest.get_us_time() for i = 1,20000 do for k, v in ipairs(ttt) do print(v) end end minetest.chat_send_all("Ipairs: "..tonumber(minetest.get_us_time()-time)) local time = minetest.get_us_time() for i = 1,20000 do for i = 1, #ttt do print(ttt[i]) end end minetest.chat_send_all("#: "..tonumber(minetest.get_us_time()-time)) end) ``` And I got: Pairs: 137000 Ipairs: 57000 for i =.... : 75000

AFCMS commented

2021-03-16 13:37:51 +01:00

@kay27 Can lua version affect pairs/ipairs time of execution?
For me:

Minetest 5.4.0 (Windows)
Using Irrlicht 1.8.4
Using LuaJIT 2.1.0-beta3
BUILD_TYPE=Release
RUN_IN_PLACE=1
USE_CURL=1
USE_GETTEXT=1
USE_SOUND=1
USE_FREETYPE=1
STATIC_SHAREDIR="."
STATIC_LOCALEDIR="locale"

@kay27 Can lua version affect pairs/ipairs time of execution? For me: ``` Minetest 5.4.0 (Windows) Using Irrlicht 1.8.4 Using LuaJIT 2.1.0-beta3 BUILD_TYPE=Release RUN_IN_PLACE=1 USE_CURL=1 USE_GETTEXT=1 USE_SOUND=1 USE_FREETYPE=1 STATIC_SHAREDIR="." STATIC_LOCALEDIR="locale" ```

kay27 commented

2021-03-16 13:39:34 +01:00

I also think you put the CRITICAL label when you meant to put high priority. Among the labels CRITICAL is described as "for crashes, catastrophic bugs" while high priority is described as "for the most important issues".

Thanks for pointing me.
Yep, it was kinda a panic because some functions with iterations call other functions from these iterations and these callee contain iterations as well so I think it might cause logarithmic slowdown.
But as I currently not absolutely sure in this I'll better have removed the label

> I also think you put the *CRITICAL* label when you meant to put *high priority*. Among the labels *CRITICAL* is described as "for crashes, catastrophic bugs" while *high priority* is described as "for the most important issues". Thanks for pointing me. Yep, it was kinda a panic because some functions with iterations call other functions from these iterations and these callee contain iterations as well so I think it might cause logarithmic slowdown. But as I currently not absolutely sure in this I'll better have removed the label

kay27 added

#P3: elevated

and removed

#P1 CRITICAL

labels 2021-03-16 13:39:50 +01:00

AFCMS commented

2021-03-16 13:41:00 +01:00

If these changes really affects performances, I will be happy to change them back

kay27 commented

2021-03-16 13:42:33 +01:00

@kay27 ??????????????????
I don't understand.

minetest.register_on_joinplayer(function(player) 
	local ttt = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"}
	local out = {}
	local time = minetest.get_us_time()
	for i = 1,20000 do
		for k, v in pairs(ttt) do print(v) end
	end
	minetest.chat_send_all("Pairs:  "..tonumber(minetest.get_us_time()-time))
	local time = minetest.get_us_time()
	for i = 1,20000 do
		for k, v in ipairs(ttt) do print(v) end
	end
	minetest.chat_send_all("Ipairs:  "..tonumber(minetest.get_us_time()-time))
	local time = minetest.get_us_time()
	for i = 1,20000 do
		for i = 1, #ttt do print(ttt[i]) end
	end
	minetest.chat_send_all("#:      "..tonumber(minetest.get_us_time()-time))
end)

And I got:
Pairs: 137000
Ipairs: 57000
for i =.... : 75000

on_joinplayer means for me that this function calls only once when somebody joins the server
this is a special moment on the timeline, where the server becomes a little more busy than usual to store new connection in memory, make initial calculations, etc
so i wouldn't better measure at this time at all

> @kay27 ?????????????????? > I don't understand. > ``` > minetest.register_on_joinplayer(function(player) > local ttt = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"} > local out = {} > local time = minetest.get_us_time() > for i = 1,20000 do > for k, v in pairs(ttt) do print(v) end > end > minetest.chat_send_all("Pairs: "..tonumber(minetest.get_us_time()-time)) > local time = minetest.get_us_time() > for i = 1,20000 do > for k, v in ipairs(ttt) do print(v) end > end > minetest.chat_send_all("Ipairs: "..tonumber(minetest.get_us_time()-time)) > local time = minetest.get_us_time() > for i = 1,20000 do > for i = 1, #ttt do print(ttt[i]) end > end > minetest.chat_send_all("#: "..tonumber(minetest.get_us_time()-time)) > end) > ``` > And I got: > Pairs: 137000 > Ipairs: 57000 > for i =.... : 75000 on_joinplayer means for me that this function calls only once when somebody joins the server this is a special moment on the timeline, where the server becomes a little more busy than usual to store new connection in memory, make initial calculations, etc so i wouldn't better measure at this time at all

kay27 commented

2021-03-16 13:44:50 +01:00

If these changes really affects performances, I will be happy to change them back

please check my test code, i might be wrong too
but common practise of making lua work faster seems to be ~~using pairs still~~
ooops, sorry, of course accessing using integer indexes in for loops (but i need to check if it suitable for objects like these) and pairs takes 2nd place
USING pairs still (see below).

> If these changes really affects performances, I will be happy to change them back please check my test code, i might be wrong too but common practise of making lua work faster seems to be ~~using pairs still~~ ~~**ooops, sorry**, of course accessing using integer indexes in ```for``` loops (but i need to check if it suitable for objects like these) and ```pairs``` takes 2nd place~~ **USING pairs still** ([see below](https://git.minetest.land/MineClone2/MineClone2/issues/1311#issuecomment-16913)).

ryvnf commented

2021-03-16 13:45:58 +01:00

@kay27 ??????????????????
I don't understand.

There are some reasons your results could be different. The second iteration where you use ipairs could have better cache utilization because it has repeated the same iteration with pairs once before.

It could also be the case that ipairs is faster than pairs depending on the ways arrays were created. If the { 1, 2, 3, 4 } notation is used Lua could maybe optimize the key layout so ipairs could be faster.

You also generally want to avoid doing I/O like print in loops when performing benchmarks (unless you want to measure the time it takes to print too).

I think kay27's benchmark is probably more accurate to real scenarios because it uses minetest.get_objects_inside_radius.

> @kay27 ?????????????????? > I don't understand. There are some reasons your results could be different. The second iteration where you use `ipairs` could have better cache utilization because it has repeated the same iteration with `pairs` once before. It could also be the case that `ipairs` is faster than `pairs` depending on the ways arrays were created. If the `{ 1, 2, 3, 4 }` notation is used Lua could maybe optimize the key layout so `ipairs` could be faster. You also generally want to avoid doing I/O like `print` in loops when performing benchmarks (unless you want to measure the time it takes to print too). I think kay27's benchmark is probably more accurate to real scenarios because it uses `minetest.get_objects_inside_radius`.

AFCMS commented

2021-03-16 14:00:24 +01:00

@kay27 After trying the test from https://springrts.com/wiki/Lua_Performance#TEST_9:_for-loops, it seems that it takes a litle less time with pairs() than with ipairs(). And after trying your code, pairs seems insanely more powerfull than ipairs.

AFCMS commented

2021-03-16 14:01:22 +01:00

Also, what about for i = 1, #<table> ?

Also, what about `for i = 1, #<table>` ?

ryvnf commented

2021-03-16 14:02:31 +01:00

Try this benchmark instead. It is the same but with different arrays to minimize the effect of caching

minetest.register_chatcommand("benchmark", {
	func = function(name)
		local s = minetest.get_player_by_name(name):get_wielded_item():to_string()
		local ttt1 = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"}
		local ttt2 = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"}
		local ttt3 = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"}
		local out = {}
		local time = minetest.get_us_time()
		for i = 1,20000 do
			for k, v in pairs(ttt2) do print(v) end
		end
		minetest.chat_send_all("Pairs:  "..tonumber(minetest.get_us_time()-time))
		local time = minetest.get_us_time()
		for i = 1,20000 do
			for k, v in ipairs(ttt1) do print(v) end
		end
		minetest.chat_send_all("Ipairs:  "..tonumber(minetest.get_us_time()-time))
		local time = minetest.get_us_time()
		for i = 1,20000 do
			for i = 1, #ttt3 do print(ttt3[i]) end
		end
		minetest.chat_send_all("#:      "..tonumber(minetest.get_us_time()-time))
		return true, s
	end
})

Then I get

Pairs:  3307899
Ipairs: 3954348
#:      3319681

Try this benchmark instead. It is the same but with different arrays to minimize the effect of caching ```lua minetest.register_chatcommand("benchmark", { func = function(name) local s = minetest.get_player_by_name(name):get_wielded_item():to_string() local ttt1 = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"} local ttt2 = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"} local ttt3 = {"sfdfsfs","sdfdsff","sfdfs",12,"feff","sfdfsfs"} local out = {} local time = minetest.get_us_time() for i = 1,20000 do for k, v in pairs(ttt2) do print(v) end end minetest.chat_send_all("Pairs: "..tonumber(minetest.get_us_time()-time)) local time = minetest.get_us_time() for i = 1,20000 do for k, v in ipairs(ttt1) do print(v) end end minetest.chat_send_all("Ipairs: "..tonumber(minetest.get_us_time()-time)) local time = minetest.get_us_time() for i = 1,20000 do for i = 1, #ttt3 do print(ttt3[i]) end end minetest.chat_send_all("#: "..tonumber(minetest.get_us_time()-time)) return true, s end }) ``` Then I get ``` Pairs: 3307899 Ipairs: 3954348 #: 3319681 ```

AFCMS commented

2021-03-16 14:11:04 +01:00

@ryvnf I got:

It is pretty different from your results.

@ryvnf I got: ![image](/attachments/629ace3b-3ded-44ef-8593-149ebbe740c9) It is pretty different from your results.

image.png

12 KiB

ryvnf commented

2021-03-16 14:19:30 +01:00

Also, what about for i = 1, #<table>?

I think ipairs should be preferred for code readability unless the code is performance critical. Most of the time it will not matter.

Also note that pairs is different from ipairs and # because it will also go through non-numeric keys.

> Also, what about `for i = 1, #<table>`? I think `ipairs` should be preferred for code readability unless the code is performance critical. Most of the time it will not matter. Also note that `pairs` is different from `ipairs` and `#` because it will also go through non-numeric keys.

ryvnf commented

2021-03-16 14:23:12 +01:00

@AFCMS That is suprising.

Could you also try to swap the order of the pairs and ipairs loop? (to see if there is still some caching going on there)

@AFCMS That is suprising. Could you also try to swap the order of the `pairs` and `ipairs` loop? (to see if there is still some caching going on there)

kneekoo commented

2021-03-16 14:30:34 +01:00

I did multiple tests about 1 minute apart.

Test	Pairs	Ipairs	#
1	96630	106784	130100
2	138622	93322	98484
3	99928	104186	135304
4	101184	104689	141557
5	145653	94043	101153
6	105485	102030	130625
7	102711	146512	97151
8	154517	93672	101506
9	154713	96064	101774
10	97237	106954	160535

Ipairs looks quite consistent, with the exception of test 7.

I did multiple tests about 1 minute apart. | Test | Pairs | Ipairs | # | | --- | --- | --- | --- | | 1 | 96630 | 106784 | 130100 | | 2 | 138622 | 93322 | 98484 | | 3 | 99928 | 104186 | 135304 | | 4 | 101184 | 104689 | 141557 | | 5 | 145653 | 94043 | 101153 | | 6 | 105485 | 102030 | 130625 | | 7 | 102711 | 146512 | 97151 | | 8 | 154517 | 93672 | 101506 | |9 | 154713 | 96064 | 101774 | | 10 | 97237 | 106954 | 160535 | Ipairs looks quite consistent, with the exception of test 7.

kay27 commented

2021-03-16 15:30:22 +01:00

I did multiple tests about 1 minute apart.

This test uses print() which maybe depends on outer ~~irrlicht~~freetype so for me it's more likely yet another random noise generator :)

And why don't you like my test? :)

I've taken additional steps to make equal measurements. Also I've added the linear for, but still not sure if it works as expected.

The idea of the test is: you move, and enumerate the objects around you. So the more objects you have around you - the better result you get.

And the time divides by the number of object to make it always fair.

And seems the docs (and I) were wrong about linear for.

And there is an easy way to check if my test is right - just swap pairs and ipairs in first two functions - and you should get some comparable mixed averaged results for them - let's see:

Test code works like a charm, I confirm that.

Linear for works better here because pairs and ipairs are mixed and the result is average result of them both.

We always should better use paris instead of ipairs expect maybe the cases we need to reindex the table.

Final test code (sorry for the size, I've added it to the end of mods\ENVIRONMENT\mcl_weather\weather_core.lua after load_weather(), you're free to find better place):

local runtest

local goir = minetest.get_objects_inside_radius
local gust = minetest.get_us_time
local player, pos
local ppp, ppn = 0, 0
local function qwe_p()
	local objs = goir(pos, 25)
	if not objs then
		minetest.log("warning", "test has no objects")
		return
	end
	local n = #objs
	if n < 2 then
		minetest.log("warning", "test has too few objects")
		return
	end
	local t1 = gust()
	for _, o in pairs(objs) do
		local a = o
	end
	local t2 = gust()
	local dt = t2-t1
	local dt_per_n = dt/n
	ppp = ppp + dt_per_n
	ppn = ppn + 1
end
local iii, iin = 0, 0
local function qwe_i()
	local objs = goir(pos, 25)
	if not objs then
		minetest.log("warning", "test has no objects")
		return
	end
	local n = #objs
	if n < 2 then
		minetest.log("warning", "test has too few objects")
		return
	end
	local t1 = gust()
	for _, o in ipairs(objs) do
		local a = o
	end
	local t2 = gust()
	local dt = t2-t1
	local dt_per_n = dt/n
	iii = iii + dt_per_n
	iin = iin + 1
end
local function qwe_i2()
	local objs = goir(pos, 25)
	if not objs then
		minetest.log("warning", "test has no objects")
		return
	end
	local n = #objs
	if n < 2 then
		minetest.log("warning", "test has too few objects")
		return
	end
	local t1 = gust()
	for _, o in ipairs(objs) do
		local a = o
	end
	local t2 = gust()
	local dt = t2-t1
	local dt_per_n = dt/n
	iii = iii + dt_per_n
	iin = iin + 1
end
local function qwe_p2()
	local objs = goir(pos, 25)
	if not objs then
		minetest.log("warning", "test has no objects")
		return
	end
	local n = #objs
	if n < 2 then
		minetest.log("warning", "test has too few objects")
		return
	end
	local t1 = gust()
	for _, o in pairs(objs) do
		local a = o
	end
	local t2 = gust()
	local dt = t2-t1
	local dt_per_n = dt/n
	ppp = ppp + dt_per_n
	ppn = ppn + 1
end
local nnn, nnm = 0, 0
local function qwe_n1()
	local objs = goir(pos, 25)
	if not objs then
		minetest.log("warning", "test has no objects")
		return
	end
	local n = #objs
	if n < 2 then
		minetest.log("warning", "test has too few objects")
		return
	end
	local t1 = gust()
	for i = 1, n do
		local a = objs[i]
	end
	local t2 = gust()
	local dt = t2-t1
	local dt_per_n = dt/n
	nnn = nnn + dt_per_n
	nnm = nnm + 1
end
local function qwe_n2()
	local objs = goir(pos, 25)
	if not objs then
		minetest.log("warning", "test has no objects")
		return
	end
	local n = #objs
	if n < 2 then
		minetest.log("warning", "test has too few objects")
		return
	end
	local t1 = gust()
	for i = 1, n do
		local a = objs[i]
	end
	local t2 = gust()
	local dt = t2-t1
	local dt_per_n = dt/n
	nnn = nnn + dt_per_n
	nnm = nnm + 1
end

local functions = { qwe_i, qwe_p, qwe_p2, qwe_i2, qwe_n1, qwe_n2 }

local function stats()
	minetest.chat_send_all("paris: "..tostring(ppp).."/"..tostring(ppn).."="..tostring(ppp/ppn))
	minetest.chat_send_all("iparis: "..tostring(iii).."/"..tostring(iin).."="..tostring(iii/iin))
	minetest.chat_send_all("#for: "..tostring(nnn).."/"..tostring(nnm).."="..tostring(nnn/nnm))
	if not player then
		runtest = false
	else
		pos = player:get_pos()
		minetest.after(1, stats)
	end
end
local function tttest()
	functions[math.random(1,#functions)]()
	if runtest then
		minetest.after(0.01, tttest)
	else
		minetest.chat_send_all("testing loop finished")
	end
end
minetest.register_chatcommand("test", {
	params = "",
	description = S("Performing a test."),
	privs = {},
	func = function(name, param)
		player = minetest.get_player_by_name(name)
		if not player then
			return false
		end
		pos = player:get_pos()

		runtest = true
		minetest.chat_send_all("entering testing loop")
		minetest.after(0.1, tttest)
		minetest.after(1, stats)
		return true
	end
})
minetest.register_chatcommand("stop", {
	params = "",
	description = S("Stopping the test."),
	privs = {},
	func = function(name, param)
		runtest = false
		return true
	end
})

> I did multiple tests about 1 minute apart. This test uses print() which maybe depends on outer ~~irrlicht~~**freetype** so for me it's more likely yet another random noise generator :) And why don't you like my test? :) I've taken additional steps to make equal measurements. Also I've added the linear for, but still not sure if it works as expected. The idea of the test is: you move, and enumerate the objects around you. So the more objects you have around you - the better result you get. And the time divides by the number of object to make it always fair. And seems the docs (and I) were wrong about ```linear for```. ![изображение](/attachments/c32f5896-8909-4783-a6aa-78fb6144a7c9) And there is an easy way to check if my test is right - just swap ```pairs``` and ```ipairs``` in first two functions - and you should get some comparable mixed averaged results for them - let's see: ![изображение](/attachments/e6a1b0d3-ab90-4396-886d-05cdd9e84945) Test code works like a charm, I confirm that. ```Linear for``` works better here because pairs and ipairs are mixed and the result is average result of them both. We always should better use ```paris``` instead of ```ipairs``` expect maybe the cases we need to reindex the table. Final test code (sorry for the size, I've added it to the end of ```mods\ENVIRONMENT\mcl_weather\weather_core.lua``` after ```load_weather()```, you're free to find better place): ```lua local runtest local goir = minetest.get_objects_inside_radius local gust = minetest.get_us_time local player, pos local ppp, ppn = 0, 0 local function qwe_p() local objs = goir(pos, 25) if not objs then minetest.log("warning", "test has no objects") return end local n = #objs if n < 2 then minetest.log("warning", "test has too few objects") return end local t1 = gust() for _, o in pairs(objs) do local a = o end local t2 = gust() local dt = t2-t1 local dt_per_n = dt/n ppp = ppp + dt_per_n ppn = ppn + 1 end local iii, iin = 0, 0 local function qwe_i() local objs = goir(pos, 25) if not objs then minetest.log("warning", "test has no objects") return end local n = #objs if n < 2 then minetest.log("warning", "test has too few objects") return end local t1 = gust() for _, o in ipairs(objs) do local a = o end local t2 = gust() local dt = t2-t1 local dt_per_n = dt/n iii = iii + dt_per_n iin = iin + 1 end local function qwe_i2() local objs = goir(pos, 25) if not objs then minetest.log("warning", "test has no objects") return end local n = #objs if n < 2 then minetest.log("warning", "test has too few objects") return end local t1 = gust() for _, o in ipairs(objs) do local a = o end local t2 = gust() local dt = t2-t1 local dt_per_n = dt/n iii = iii + dt_per_n iin = iin + 1 end local function qwe_p2() local objs = goir(pos, 25) if not objs then minetest.log("warning", "test has no objects") return end local n = #objs if n < 2 then minetest.log("warning", "test has too few objects") return end local t1 = gust() for _, o in pairs(objs) do local a = o end local t2 = gust() local dt = t2-t1 local dt_per_n = dt/n ppp = ppp + dt_per_n ppn = ppn + 1 end local nnn, nnm = 0, 0 local function qwe_n1() local objs = goir(pos, 25) if not objs then minetest.log("warning", "test has no objects") return end local n = #objs if n < 2 then minetest.log("warning", "test has too few objects") return end local t1 = gust() for i = 1, n do local a = objs[i] end local t2 = gust() local dt = t2-t1 local dt_per_n = dt/n nnn = nnn + dt_per_n nnm = nnm + 1 end local function qwe_n2() local objs = goir(pos, 25) if not objs then minetest.log("warning", "test has no objects") return end local n = #objs if n < 2 then minetest.log("warning", "test has too few objects") return end local t1 = gust() for i = 1, n do local a = objs[i] end local t2 = gust() local dt = t2-t1 local dt_per_n = dt/n nnn = nnn + dt_per_n nnm = nnm + 1 end local functions = { qwe_i, qwe_p, qwe_p2, qwe_i2, qwe_n1, qwe_n2 } local function stats() minetest.chat_send_all("paris: "..tostring(ppp).."/"..tostring(ppn).."="..tostring(ppp/ppn)) minetest.chat_send_all("iparis: "..tostring(iii).."/"..tostring(iin).."="..tostring(iii/iin)) minetest.chat_send_all("#for: "..tostring(nnn).."/"..tostring(nnm).."="..tostring(nnn/nnm)) if not player then runtest = false else pos = player:get_pos() minetest.after(1, stats) end end local function tttest() functions[math.random(1,#functions)]() if runtest then minetest.after(0.01, tttest) else minetest.chat_send_all("testing loop finished") end end minetest.register_chatcommand("test", { params = "", description = S("Performing a test."), privs = {}, func = function(name, param) player = minetest.get_player_by_name(name) if not player then return false end pos = player:get_pos() runtest = true minetest.chat_send_all("entering testing loop") minetest.after(0.1, tttest) minetest.after(1, stats) return true end }) minetest.register_chatcommand("stop", { params = "", description = S("Stopping the test."), privs = {}, func = function(name, param) runtest = false return true end }) ```

изображение.png

116 KiB

изображение.png

104 KiB

AFCMS commented

2021-03-16 15:52:58 +01:00

@kay27 I got that with your code but I don't understand the result:

EDIT: Also, pairs() is different from paris() xD

@kay27 I got that with your code but I don't understand the result: ![image](/attachments/fc060e36-baeb-425a-85d1-313079b9f4db) EDIT: Also, pairs() is different from paris() xD

image.png

29 KiB

kay27 commented

2021-03-16 16:07:50 +01:00

@kay27 I got that with your code but I don't understand the result:

EDIT: Also, pairs() is different from paris() xD

Please add/meet some objects around you, eg. spawn some harmless mobs.
The result is: all the time divided by the number of measurements equals average time per operation in microseconds.
But 25, 24 and 29 are too small values to get perfect result, more objects will help getting it more accurate

> @kay27 I got that with your code but I don't understand the result: > ![image](/attachments/fc060e36-baeb-425a-85d1-313079b9f4db) > > EDIT: Also, pairs() is different from paris() xD Please add/meet some objects around you, eg. spawn some harmless mobs. The result is: all the time divided by the number of measurements equals average time per operation in microseconds. But 25, 24 and 29 are too small values to get perfect result, more objects will help getting it more accurate

AFCMS commented

2021-03-16 16:59:32 +01:00

@kay27 I got that with your code but I don't understand the result:

EDIT: Also, pairs() is different from paris() xD

Please add/meet some objects around you, eg. spawn some harmless mobs.
The result is: all the time divided by the number of measurements equals average time per operation in microseconds.
But 25, 24 and 29 are too small values to get perfect result, more objects will help getting it more accurate

Will try soon.

> > @kay27 I got that with your code but I don't understand the result: > > ![image](/attachments/fc060e36-baeb-425a-85d1-313079b9f4db) > > > > EDIT: Also, pairs() is different from paris() xD > > Please add/meet some objects around you, eg. spawn some harmless mobs. > The result is: all the time divided by the number of measurements equals average time per operation in microseconds. > But 25, 24 and 29 are too small values to get perfect result, more objects will help getting it more accurate Will try soon.

AFCMS commented

2021-03-16 17:28:23 +01:00

@kay27
Is there enough mobs? :-)

Result:

@kay27 Is there enough mobs? :-) Result: ![image](/attachments/2f3b64e0-d771-4f20-afc5-075030067ef6)

image.png

29 KiB

screenshot_20210316_172516.png

1.9 MiB

AFCMS commented

2021-03-16 17:29:19 +01:00

It seems pairs is clearly superior to ipairs....
I will change all back

It seems pairs is clearly superior to ipairs.... I will change all back

AFCMS commented

2021-03-16 17:43:56 +01:00

Fixed!

AFCMS closed this issue

2021-03-16 17:43:58 +01:00

kay27 commented

2021-03-16 23:23:01 +01:00

@kay27
Is there enough mobs? :-)

Result:

Not sure if here were enough mobs around you because I've forgotten to say that they should be at maximum distance of 25 nodes for this test. But even the approximate result you've got seems to be correct.

Limit of 25 has some reasons, related to map chunks. But it can be increased in every of 6 function

> @kay27 > Is there enough mobs? :-) > > > Result: > ![image](/attachments/2f3b64e0-d771-4f20-afc5-075030067ef6) Not sure if here were enough mobs around you because I've forgotten to say that they should be at maximum distance of 25 nodes for this test. But even the approximate result you've got seems to be correct. Limit of 25 has some reasons, related to map chunks. But it can be increased in every of 6 function

kay27 commented

2021-03-16 23:45:03 +01:00

FYI:
You can use git revert <COMMIT> to revert some commit.
There are now several places with pairs where previously more likely was iparis and key variable is meaningful, it's not underscore, moreover, it is used further sometimes:

+++ /mods/ENVIRONMENT/mcl_weather/skycolor.lua
@@ -43,7 +43,7 @@ mcl_weather.skycolor = {
 
 	-- Remove layer from colors table
 	remove_layer = function(layer_name)
-		for k, name in ipairs(mcl_weather.skycolor.layer_names) do
+		for k, name in pairs(mcl_weather.skycolor.layer_names) do
 			if name == layer_name then
 				table.remove(mcl_weather.skycolor.layer_names, k)
 				mcl_weather.skycolor.force_update = true
+++ /mods/ITEMS/REDSTONE/mesecons/actionqueue.lua
@@ -22,7 +22,7 @@ function mesecon.queue:add_action(pos, func, params, time, overwritecheck, prior
 	local toremove = nil
 	-- Otherwise, add the action to the queue
 	if overwritecheck then -- check if old action has to be overwritten / removed:
-		for i, ac in ipairs(mesecon.queue.actions) do
+		for i, ac in pairs(mesecon.queue.actions) do
 			if(vector.equals(pos, ac.pos)
 			and mesecon.cmpAny(overwritecheck, ac.owcheck)) then
 				toremove = i
+++ /mods/ITEMS/mcl_beds/functions.lua
@@ -36,7 +36,7 @@ local function check_in_beds(players)
 		players = minetest.get_connected_players()
 	end
 
-	for n, player in ipairs(players) do
+	for n, player in pairs(players) do
 		local name = player:get_player_name()
 		if not in_bed[name] then
 			return false
+++ /mods/ITEMS/mcl_clock/init.lua
@@ -94,9 +94,8 @@ minetest.register_globalstep(function(dtime)
 
 	watch.old_time = now
 
-	local players = minetest.get_connected_players()
-	for p, player in ipairs(players) do
-		for s, stack in ipairs(player:get_inventory():get_list("main")) do
+	for p, player in pairs(minetest.get_connected_players()) do
+		for s, stack in pairs(player:get_inventory():get_list("main")) do
 			local dim = mcl_worlds.pos_to_dimension(player:get_pos())
 			local frame
 			-- Clocks do not work in certain zones
+++ /mods/ITEMS/mcl_compass/init.lua
@@ -14,15 +14,14 @@ local random_frame = math.random(0, compass_frames-1)
 
 minetest.register_globalstep(function(dtime)
 	random_timer = random_timer + dtime
-	local players  = minetest.get_connected_players()
 
 	if random_timer >= random_timer_trigger then
 		random_frame = (random_frame + math.random(-1, 1)) % compass_frames
 		random_timer = 0
 	end
-	for i,player in ipairs(players) do
+	for i,player in pairs(minetest.get_connected_players()) do
 		local function has_compass(player)
-			for _,stack in ipairs(player:get_inventory():get_list("main")) do
+			for _,stack in pairs(player:get_inventory():get_list("main")) do
 				if minetest.get_item_group(stack:get_name(), "compass") ~= 0 then
 					return true
 				end
@@ -53,7 +52,7 @@ minetest.register_globalstep(function(dtime)
 				compass_image = math.floor((angle_relative/11.25) + 0.5) % compass_frames
 			end
 
-			for j,stack in ipairs(player:get_inventory():get_list("main")) do
+			for j,stack in pairs(player:get_inventory():get_list("main")) do
 				if minetest.get_item_group(stack:get_name(), "compass") ~= 0 and
 						minetest.get_item_group(stack:get_name(), "compass")-1 ~= compass_image then
 					local itemname = "mcl_compass:"..compass_image

I hope it will be all right with this code.
But of course it would be better to check.

FYI: You can use ```git revert <COMMIT>``` to revert some commit. There are now several places with ```pairs``` where previously more likely was ```iparis``` and key variable is meaningful, it's not underscore, moreover, it is used further sometimes: ```diff +++ /mods/ENVIRONMENT/mcl_weather/skycolor.lua @@ -43,7 +43,7 @@ mcl_weather.skycolor = { -- Remove layer from colors table remove_layer = function(layer_name) - for k, name in ipairs(mcl_weather.skycolor.layer_names) do + for k, name in pairs(mcl_weather.skycolor.layer_names) do if name == layer_name then table.remove(mcl_weather.skycolor.layer_names, k) mcl_weather.skycolor.force_update = true +++ /mods/ITEMS/REDSTONE/mesecons/actionqueue.lua @@ -22,7 +22,7 @@ function mesecon.queue:add_action(pos, func, params, time, overwritecheck, prior local toremove = nil -- Otherwise, add the action to the queue if overwritecheck then -- check if old action has to be overwritten / removed: - for i, ac in ipairs(mesecon.queue.actions) do + for i, ac in pairs(mesecon.queue.actions) do if(vector.equals(pos, ac.pos) and mesecon.cmpAny(overwritecheck, ac.owcheck)) then toremove = i +++ /mods/ITEMS/mcl_beds/functions.lua @@ -36,7 +36,7 @@ local function check_in_beds(players) players = minetest.get_connected_players() end - for n, player in ipairs(players) do + for n, player in pairs(players) do local name = player:get_player_name() if not in_bed[name] then return false +++ /mods/ITEMS/mcl_clock/init.lua @@ -94,9 +94,8 @@ minetest.register_globalstep(function(dtime) watch.old_time = now - local players = minetest.get_connected_players() - for p, player in ipairs(players) do - for s, stack in ipairs(player:get_inventory():get_list("main")) do + for p, player in pairs(minetest.get_connected_players()) do + for s, stack in pairs(player:get_inventory():get_list("main")) do local dim = mcl_worlds.pos_to_dimension(player:get_pos()) local frame -- Clocks do not work in certain zones +++ /mods/ITEMS/mcl_compass/init.lua @@ -14,15 +14,14 @@ local random_frame = math.random(0, compass_frames-1) minetest.register_globalstep(function(dtime) random_timer = random_timer + dtime - local players = minetest.get_connected_players() if random_timer >= random_timer_trigger then random_frame = (random_frame + math.random(-1, 1)) % compass_frames random_timer = 0 end - for i,player in ipairs(players) do + for i,player in pairs(minetest.get_connected_players()) do local function has_compass(player) - for _,stack in ipairs(player:get_inventory():get_list("main")) do + for _,stack in pairs(player:get_inventory():get_list("main")) do if minetest.get_item_group(stack:get_name(), "compass") ~= 0 then return true end @@ -53,7 +52,7 @@ minetest.register_globalstep(function(dtime) compass_image = math.floor((angle_relative/11.25) + 0.5) % compass_frames end - for j,stack in ipairs(player:get_inventory():get_list("main")) do + for j,stack in pairs(player:get_inventory():get_list("main")) do if minetest.get_item_group(stack:get_name(), "compass") ~= 0 and minetest.get_item_group(stack:get_name(), "compass")-1 ~= compass_image then local itemname = "mcl_compass:"..compass_image ``` I hope it will be all right with this code. But of course it would be better to check.

AFCMS commented

2021-03-17 09:03:14 +01:00

@kay27
Is there enough mobs? :-)

Result:

Not sure if here were enough mobs around you because I've forgotten to say that they should be at maximum distance of 25 nodes for this test. But even the approximate result you've got seems to be correct.

Limit of 25 has some reasons, related to map chunks. But it can be increased in every of 6 function

Did you see the screenshot?

> > @kay27 > > Is there enough mobs? :-) > > > > > > Result: > > ![image](/attachments/2f3b64e0-d771-4f20-afc5-075030067ef6) > > Not sure if here were enough mobs around you because I've forgotten to say that they should be at maximum distance of 25 nodes for this test. But even the approximate result you've got seems to be correct. > > Limit of 25 has some reasons, related to map chunks. But it can be increased in every of 6 function Did you see the screenshot?

kay27 changed title from ~~@AFCMS Change ipairs() back to pairs()~~ to Change ipairs() back to pairs()

2021-05-10 23:00:42 +02:00

kay27 referenced this issue from Mineclonia/Mineclonia

2021-05-10 23:07:16 +02:00

[ITEMS/mcl_portals] Backport Nether portal changes from Mineclone2 post-0.71 #58

kay27 referenced this issue

2022-07-28 20:37:21 +02:00

Cactus deals damage incorrectly #2484

Sign in to join this conversation.