Profiler issue raised for raids #3077

New Issue

ancientmarinerdev · 2022-12-06T14:28:32+01:00

ancientmarinerdev commented

2022-12-06 14:28:32 +01:00

A minetest dev raised a profiler spike for raids. It's worth considering if we can tweak this slightly.

@PrairieAstronomer Any ideas? If not, I can try to check it out later.

A minetest dev raised a profiler spike for raids. It's worth considering if we can tweak this slightly. @PrairieAstronomer Any ideas? If not, I can try to check it out later.

profiler issue raids.png

62 KiB

👍 1

ancientmarinerdev added the

mobs

performance

labels 2022-12-06 14:28:50 +01:00

rollerozxa commented

2022-12-06 15:26:13 +01:00

Here's the full interactive profiler graph (mcl2 commit 826b9fcc45, minetest 5.7.0-dev, profiled on a pretty much new world) if anyone wants to check: https://grejer.voxelmanip.se/uploads/mcl2_001.svg

Most importantly, it looks like mcl_raids.find_bed and/or mcl_raids.find_villager being called by mcl_raids.find_village has some kind of bottleneck.

Here's the full interactive profiler graph (mcl2 commit 826b9fcc45e4, minetest 5.7.0-dev, profiled on a pretty much new world) if anyone wants to check: https://grejer.voxelmanip.se/uploads/mcl2_001.svg Most importantly, it looks like `mcl_raids.find_bed` and/or `mcl_raids.find_villager` being called by `mcl_raids.find_village` has some kind of bottleneck.

ancientmarinerdev commented

2022-12-08 22:05:29 +01:00

@rollerozxa analysis is spot on here. I suspect it's find bed:

function mcl_raids.find_bed(pos)
	return minetest.find_node_near(pos,128,{"mcl_beds:bed_red_bottom"})
end

function mcl_raids.find_village(pos)
	local bed = mcl_raids.find_bed(pos)
	if bed and mcl_raids.find_villager(bed) then
		return bed
	end
end

It's looking 128 blocks! I'm wondering if that's looking in all directions, cause that's gonna be expensive.

I'm wondering if we just find beds a certain distance from the town bell. That could make that computationally less expensive, because village structures spawn from near the bell. If the raid starts near the bell, or heads to the bell and goes from there, it could be efficient.

@rollerozxa analysis is spot on here. I suspect it's find bed: ``` function mcl_raids.find_bed(pos) return minetest.find_node_near(pos,128,{"mcl_beds:bed_red_bottom"}) end function mcl_raids.find_village(pos) local bed = mcl_raids.find_bed(pos) if bed and mcl_raids.find_villager(bed) then return bed end end ``` It's looking 128 blocks! I'm wondering if that's looking in all directions, cause that's gonna be expensive. I'm wondering if we just find beds a certain distance from the town bell. That could make that computationally less expensive, because village structures spawn from near the bell. If the raid starts near the bell, or heads to the bell and goes from there, it could be efficient.

Michieal commented

2022-12-09 02:25:48 +01:00

Question: wouldn't finding the town bell be as computationally expensive? or am I missing something here?

ancientmarinerdev commented

2022-12-09 02:43:59 +01:00

Question: wouldn't finding the town bell be as computationally expensive? or am I missing something here?

Maybe you're right. I don't think I engaged brain fully at that point.

For most stuff villager related, we check within 48 blocks and it isn't that bad. Perhaps we shouldn't look to start a raid more than 48 squares away from a player. It feels a bit excessive, and if that's 48 squares in all directions, that's a lotta squares.

Because you need to be near the village for this to be meaningful, maybe 32/38 squares from town bell would make sense.

> Question: wouldn't finding the town bell be as computationally expensive? or am I missing something here? Maybe you're right. I don't think I engaged brain fully at that point. For most stuff villager related, we check within 48 blocks and it isn't that bad. Perhaps we shouldn't look to start a raid more than 48 squares away from a player. It feels a bit excessive, and if that's 48 squares in all directions, that's a lotta squares. Because you need to be near the village for this to be meaningful, maybe 32/38 squares from town bell would make sense.

Michieal commented

2022-12-09 03:11:07 +01:00

32m away would be a lot better. (1/16th the scanning area of 128 -- like image size, cutting it in half reduces it by a factor of 4.)

👍 1

ancientmarinerdev commented

2022-12-09 10:44:06 +01:00

This is profiler results from one of our players who's server slowed down. Over 50% on zombie siege finding a village.

I will fix this this weekend and test. I will also ensure it doesn't check for village if it's not the correct time, so it doesn't do this processing at all during the day.

This is profiler results from one of our players who's server slowed down. Over 50% on zombie siege finding a village. I will fix this this weekend and test. I will also ensure it doesn't check for village if it's not the correct time, so it doesn't do this processing at all during the day.

graph.svg

174 KiB

👍 1 ❤️ 1

Michieal commented

2022-12-09 11:13:52 +01:00

Fyi... multiple zombie raids can be started (so, probably pillager raids too)... So... Need to add in a zombie raid start check, to see if a raid is already started... and if it is, disallow starting a new one.

I crashed my game trying to test it out...

Fyi... multiple zombie raids can be started (so, probably pillager raids too)... So... Need to add in a zombie raid start check, to see if a raid is already started... and if it is, disallow starting a new one. I crashed my game trying to test it out...

👍 2

ancientmarinerdev added the

#P3: elevated

label 2022-12-09 15:25:31 +01:00

ancientmarinerdev added this to the 0.82.0 - Bamburgered milestone 2022-12-09 15:25:43 +01:00

Ghost commented

2022-12-09 18:42:53 +01:00

I would say the distance should probably be the same as the villagers check, which I believe is 16 nodes.

👍 1

Ghost commented

2022-12-09 18:43:16 +01:00

I dont recall making it 128, but I could be wrong.

Ghost commented

2022-12-09 18:55:20 +01:00

Well, I found the commit that messed this up, 37144f8787.

Well, I found the commit that messed this up, 37144f8787.

Michieal commented

2022-12-10 04:43:35 +01:00

Also, from discord:

JoMW1998 — Today Hi I just came back and saw that mineclone 2 just crashed with this error AsyncErr: Lua: Runtime error from mod '??' in callback environment_Step(): ...s\mineclone2\mods\ENVIRONMENT\mcl_zombie_sieges\init.lua:13: attempt to index local 'm' (a nil value) stack traceback: ...s\mineclone2\mods\ENVIRONMENT\mcl_zombie_sieges\init.lua:13: in function 'on_stage_begin' ...64\bin..\games\mineclone2\mods\CORE\mcl_events\init.lua:85: in function <...64\bin..\games\mineclone2\mods\CORE\mcl_events\init.lua:72> F:\minetest-5.6.1-win64\bin..\builtin\game\register.lua:431: in function <F:\minetest-5.6.1-win64\bin..\builtin\game\register.lua:417>

I moved away from the village and the error stopped but when coming back to the same village it re appears with the same error and i have peaceful mode on `

Thought that it should be known.

Also, from discord: `JoMW1998 — Today Hi I just came back and saw that mineclone 2 just crashed with this error AsyncErr: Lua: Runtime error from mod '??' in callback environment_Step(): ...s\mineclone2\mods\ENVIRONMENT\mcl_zombie_sieges\init.lua:13: attempt to index local 'm' (a nil value) stack traceback: ...s\mineclone2\mods\ENVIRONMENT\mcl_zombie_sieges\init.lua:13: in function 'on_stage_begin' ...64\bin..\games\mineclone2\mods\CORE\mcl_events\init.lua:85: in function <...64\bin..\games\mineclone2\mods\CORE\mcl_events\init.lua:72> F:\minetest-5.6.1-win64\bin..\builtin\game\register.lua:431: in function <F:\minetest-5.6.1-win64\bin..\builtin\game\register.lua:417>` I moved away from the village and the error stopped but when coming back to the same village it re appears with the same error and i have peaceful mode on ` Thought that it should be known.

Michieal referenced this issue

2022-12-10 05:23:26 +01:00

performance_fix_raids #3092

kabou commented

2022-12-10 13:03:42 +01:00

Thought that it should be known.

Looks like a different crash than the one fixed in #3063 .

What mineclone version is this user running, current git?

Please submit a new issue for this if the user cannot do it himself.

> Thought that it should be known. Looks like a different crash than the one fixed in #3063 . What mineclone version is this user running, current git? Please submit a new issue for this if the user cannot do it himself.

kabou commented

2022-12-10 13:12:20 +01:00

I guess we need to add another check-object-before-use there.

ancientmarinerdev referenced this issue

2022-12-10 18:06:04 +01:00

performance_fix_raids #3092

ancientmarinerdev referenced a pull request that will close this issue

2022-12-11 23:24:56 +01:00

performance_fix_raids #3092

Michieal closed this issue

2022-12-12 00:29:19 +01:00

Sign in to join this conversation.