[core] implement API alternatives for ABMs, nodetimers #2564

New Issue

Ghost · 2022-08-14T04:59:10+02:00

Ghost commented

2022-08-14 04:59:10 +02:00

problem

we and modders depend too much on ABMs and nodetimers because its easy and familiar. the result is that we use ABMs for mechanics that don't really need ABMs. this pollutes the ABMs runtime space that's better allocated for mechanics that truly needs ABMs.

examples:

kelp currently use nodetimers to detect if its stems is submerged or not. at large scales (think servers), this is going to buckle.
cactus currently use ABMs to detect if its sides have nodes so it would break. this isn't going to break anything. the issue is that accurate, fast responses would require we have a high frequency ABMs.
bubble columns mod abused ABMs for many of its mechanics.

"when all you have is a sledgehammer, everything starts to look like nails".

suggestions

we implement our own APIs for easier implementation of such mechanics. currently the following have been discussed in discord:

node_update and node_update_propagate. this would be immensely useful for many mechanics present, as many of them are triggered by players or other nodes (or automata like redstone). instead of relying on frequency of checks, we would be using callbacks to define the mechanics. @SumianVoice have written the following draft:

mcl_util.node_update(p, reason, caller)
  local node = minetest.get_node(p)
  if not node then return end
  node = minetest.registered_nodes[node.name]
  if node and node._on_update ~= nil then
    node._on_update(p, reason, caller)
end

mcl_util.node_update_adjacent(p, reason, caller)
  mcl_util.node_update(vector.offset(p, 1, 0 0), reason, caller)
  mcl_util.node_update(vector.offset(p, 0, 1 0), reason, caller)
  mcl_util.node_update(vector.offset(p, 0, 0 1), reason, caller)
  mcl_util.node_update(vector.offset(p, -1,  0  0), reason, caller)
  mcl_util.node_update(vector.offset(p,  0, -1  0), reason, caller)
  mcl_util.node_update(vector.offset(p,  0,  0 -1), reason, caller)
end

node_player_globalstep. @cora if you could say more about this that'll be great.

node updates

i believe sumi's draft should be enough to get the gist of the idea.

pros

direct, accurate, immediate. activates directly from surrounding updates and player's interaction.
no idling. basically what nodedef callbacks do but it also propagates it to nearby nodes.
scales in terms of players. being a non-idling callback, increasing player count will not pollute runtime.

cons

unreliable in edge cases → require cleanup callbacks. identifying and handling those cases mean we no longer need cleanup callbacks.
buckles when scaling in terms of propagation → updates need to be restrictively propagated. otherwise, we risk processing too many nodes.
can cause lag spikes → avoid this by possibly splitting updates to run on the next server step. probably introduces exploits. fun

alternatively, we don't split updates at all. let it update only what it can and let cleanup handle the rest.

### node updates i believe sumi's draft should be enough to get the gist of the idea. **pros** - direct, accurate, immediate. activates directly from surrounding updates and player's interaction. - no idling. basically what nodedef callbacks do but it also propagates it to nearby nodes. - scales in terms of players. being a non-idling callback, increasing player count will not pollute runtime. **cons** - unreliable in edge cases → require cleanup callbacks. identifying and handling those cases mean we no longer need cleanup callbacks. - buckles when scaling in terms of propagation → updates need to be restrictively propagated. otherwise, we risk processing too many nodes. - can cause lag spikes → avoid this by possibly splitting updates to run on the next server step. probably introduces exploits. fun alternatively, we don't split updates at all. let it update only what it can and let cleanup handle the rest.

Ghost commented

2022-08-16 10:10:01 +02:00

approximated+conditional ABMs (and other frequency/interval-based callbacks)

ABMs typically run within the radius of 4 mapblocks from the player (servers may
choose to run at smaller radii, prob 2 mapblocks). what we could do is idle ABMs
if its too far from a certain range (in nodes). we do this by: checking for the
nearest player's position and then deciding instead of doing the accurate
mechanic: no-op or execute an approximated mechanic.

to do this efficiently, we need to maintain a list of player positions so we
don't iterate through players for each running ABM.

generalising further, this radius check is just a predicate. so really, the
check could be anything as long it allows the ABM to choose between accurate
mechanics and no-op/approximated mechanics.

generalising even further, this need not be limited to ABMs, but also applicable
to nodetimers, globalstep, etc.

minetest.register_abm{
  -- everything is the same
  action = function(pos, ...)
    -- reduces radius for e.g. leaf+vine decay
    for each player:
      if pos:distance(player:get_pos()) > 8 then return end -- no-op
  end
}

pros:

easy and quick to implement as it's the least amount of work.

cons:

doesn't really solve the problem.
still polutes runtime with idling callbacks
still doesn't scale well with enough players
still depends on frequency checks → doesn't solve the core problem of abusing sledgehammer callbacks

### approximated+conditional ABMs (and other frequency/interval-based callbacks) ABMs typically run within the radius of 4 mapblocks from the player (servers may choose to run at smaller radii, prob 2 mapblocks). what we could do is idle ABMs if its too far from a certain range (in nodes). we do this by: checking for the nearest player's position and then deciding instead of doing the accurate mechanic: no-op or execute an approximated mechanic. to do this efficiently, we need to maintain a list of player positions so we don't iterate through players for each running ABM. generalising further, this radius check is just a predicate. so really, the check could be anything as long it allows the ABM to choose between accurate mechanics and no-op/approximated mechanics. generalising even further, this need not be limited to ABMs, but also applicable to nodetimers, globalstep, etc. ```lua minetest.register_abm{ -- everything is the same action = function(pos, ...) -- reduces radius for e.g. leaf+vine decay for each player: if pos:distance(player:get_pos()) > 8 then return end -- no-op end } ``` **pros**: - easy and quick to implement as it's the least amount of work. **cons**: - doesn't really solve the problem. - still polutes runtime with idling callbacks - still doesn't scale well with enough players - still depends on frequency checks → doesn't solve the core problem of abusing sledgehammer callbacks

Ghost commented

2022-08-16 10:10:28 +02:00

node globalstep → focused area manips

i think the current proposed node globalstep is kinda restrictive. we don't always want to presistently run a callback of a single node within range of a player. i think we can generalise this further into a globalstep-based API: focused area manips (FAMs). the idea is that we want to persistantly run updates within an area of a focus point, nearby a player, until (optionally) a condition is reached.

-- related APIs to consider implementing:
-- LuaEntity's on_step() but for players
mcl_proposal.register_on_stepplayer(function(player, dtime) ... end)

--- must perform vector.sort(box[1], box[2]) when constructing these instances
--- @type Box {Vector, Vector}

-- WARNING: there's an overhead from all these function constructors.
--          be sure to cache them if using repeatedly
-- WARNING: a read-only LVM is used because FAMs requires low-level, fast bulk map access
--          BUT also intended for high-level, unimpaired map write.
--          read-only as in, you cannot write to map, but you may use vm:set_*_data().
-- registers a focused area manip (FAM), even during runtime (executes on the next server step)
mcl_proposal.register_fam{
  -- see ABM definition
  label = "*pop* noice",
  chance = 1, -- (default)
  catch_up = false, -- (default)
  interval = number,

  player = function() return true end, -- (default) activates for all players
  -- activates according to player by name
  player = mcl_proposal.fam.player_name{'player1', ...},
  --- @return boolean whether to activate FAM for player or not (true → activates)
  player = function(player) ... end, -- dynamically choose players to activate

  -- (default) player as focus
  focus = mcl_proposal.fam.focus_player(),
  -- absolute or relative position as focus
  -- distance determines maximum distance of absolute position from the player to be considered as a focus
  focus = mcl_proposal.fam.focus_pos("relative", {Vector, ...}),
  focus = mcl_proposal.fam.focus_pos("absolute", distance, {Vector, ...}),
  focus = mcl_proposal.fam.focus_pos({--[[absolute]] distance, Vector, ...}, {--[[relative]] Vector, ...}), -- both
  --- @return Vector|Vector[]|nil, nil|Box|Box[]
  --- (1st return) Vector|Vector[] focus position(s) used for calculating areas
  --- (1st return) nil no focus positions found, skipping
  --- (2nd return) nil area given by area(player, fpos). see below
  --- (2nd return) Box|Box[] area(s) given for (each of) focus position(s)
  focus = function(player) ... end, -- dynamically choose focus positions

  -- area extrudes from fpos, either with equal radius in all directions or with radius defined by Vector
  area = mcl_proposal.fam.area_extrude(number|Vector),
  -- area with equal length in all directions or with length defined by Vector centers at fpos
  area = mcl_proposal.fam.area_fixed(number|Vector),
  -- area from player's position to fpos or if given, fpos:add(Vector)
  area = mcl_proposal.fam.area_from_player([Vector]),
  --- @param fpos Vector a focus position
  --- @return Box area to read from map
  area = function(player, fpos) ... end, -- dynamically choose areas

  nodenames = nil, -- allow action() to manually search instead
  nodenames = { 'nodenames' }, -- list of nodes to apply action()
  neighbors = { 'nodenames' }, -- list of node neighbors. see ABM definition

  -- see ABM definition (there's no X and Z limits for ABMs for whatevs reason)
  -- these should be hardcapped to whatever map limits mcl2 already calculated
  min_x = number, max_x = number,
  min_y = number, max_y = number,
  min_z = number, max_z = number,

  --- action() when nodenames is nil
  --- @param player ObjectRef
  --- @param focus Vector position of focus (player, position or node)
  --- @return nil|number|true
  --- number sets a temporary interval for next callback
  --- true will destroy the FAM, allowing for temporary FAMs
  action = function(player, focus) ... end

  --- action() when nodenames is a list of nodes
  --- @param player ObjectRef
  --- @param focus Vector position of focus (player, position or node)
  --- @param pos_list Vector[] positions of matching nodes
  --- @param nodes Nodes[] matching nodes
  --- @return nil|number|true
  --- number sets a temporary interval for next callback
  --- true will destroy the FAM, allowing for temporary FAMs
  action = function(player, focus, pos_list, nodes) ... end
}

-- detailed look at how FAM could be implemented -------------------------------

for each server step: for each player:
local playerpos = player:get_pos()
local fam = list of FAMs that passed the interval, chance (w catch_up) and fam.player check

-- obtain areas
local fpos, area, ids = {}, {}, {}
local f, a, tf, ta, prev_id
for each fam as fam:
  f, a = fam.focus(player)
  tf, ta = type(f), type(a)

  if not f: continue

  elseif not a and tf ~= "table":
    ids[fam.id] = (ids[prev_id] or 0)+1
    fpos:insert(f)
    area:insert(fam.area(player, f))

  elseif not a:
    ids[fam.id] = (ids[prev_id] or 0)+#f
    for each f:
      fpos:insert(each f)
      area:insert(fam.area(player, each f))

  elseif tf ~= ta: error: types of both returns from fam.focus() must be the same
  elseif tf == 'table' and #f ~= #a: error: length of both returns from fam.focus() must be equal

  else:
    fpos:add(f)
    area:add(a)
    ids[fam.id] = (ids[prev_id] or 0)+#f

  prev_id = fam.id
end
-- list of FAMs that passed the focus check (fam.focus() 1st return is non-nil)
fam = fam:filter(fam -> ids[fam.id])

-- access the map
-- TODO: solve this math problem
local boxes = calculate list of boxes to read from map, striking a balance between
              volume outside areas (wasted/unused data) and volume of total boxes
local boxes_area = list with index corresponding to an area.
                   each element is an index for boxes corresponding to a box containing an area

-- RESEARCH: make a graph of time took to read from map and cube side length using methods: minetest.get_node(), LVM
-- TODO: fallback to minetest.get_node() under areas with volume smaller than X?
local voxelmanips, cids, params, param2s = {}, {}, {}, {}, {}
for each boxes as b:
  local vm = ReadOnlyVoxelManip(b[1], b[2])
  cids:insert(vm:get_data())
  params:insert(vm:get_light_data())
  param2s:insert(vm:get_param2_data())

-- execute the active FAMs
-- pun: vi? no i use emax
-- reused variables: fpos, area, ids
local ib, ie = 1, nil
local destroy_ids = {}
local status
for each fam as fam:
  ie = ids[fam.id]
  for each (fpos:sub(ib, ie)) as f,
      each (area:sub(ib, ie)) as a,
      each (boxes_area:sub(ib, ie)) as each bi,
       each (boxes[each bi]) as box,
       each (VoxelArea(box[1], box[2])) as va,
       each (voxelManips[each bi]) as vm,
       each (cids[each bi]) as cid_data,
       each (params[each bi]) as param_data,
       each (param2s[each bi]) as param2_data:
    -- fam.node_cids → mapping of searched node content IDs to their nodename, set when registering FAM
    -- fam.neighbor_cids → mapping of neighbor content IDs to their nodename, set when registering FAM
    if fam.node_cids and fam.neighbor_cids: -- search with neighbors
      local pos_list, node_list = {}, {}
      for each (range(box[1].z, box[2].z)) as z:
      for each (range(box[1].y, box[2].y)) as y,
      for each (range(box[1].x, box[2].x)) as x,
          each (va:index(x, y, z)) as vi:
        local name = fam.node_cids[cid_data[vi]]
        if name and fam.neighbor_cids[cid_data[va:index(any 6 adjacent nodes)]]:
          pos_list:insert(vector.new(x, y, z))
          node_list:insert({node=name, param=fam.param_data[vi], param2=fam.param2_data[vi]})
      status = apply(player, f, pos_list, node)

    elseif fam.node_cids: -- search without neighbors
      local pos_list, node_list = {}, {}
      for each (range(box[1].z, box[2].z)) as z:
      for each (range(box[1].y, box[2].y)) as y,
      for each (range(box[1].x, box[2].x)) as x,
          each (va:index(x, y, z)) as vi:
        local name = fam.node_cids[cid_data[vi]]
        if name:
          pos_list:insert(vector.new(x, y, z))
          node_list:insert({node=name, param=fam.param_data[vi], param2=fam.param2_data[vi]})
      status = apply(player, f, pos_list, node)

    else: -- manual search
      status = apply(player, f, a, b, va, vm, cid_data, param_data, param2_data)

  if type(status) == "number":
    fam.tinterval = status -- temporary interval
  elseif status:
    fam.destroy_ids:insert(fam.id) -- destroy this FAM on next server step
  else:
    fam.tinterval = nil

  ib = ie+1

pros:

relatively easy to use. if you're familiar with ABMs or LBMs, this should be
very familiar.
persistent. ABMs starts dropping whatever it wants if the runtime is too
expensive. FAMs should give more control to us to prioritize, schedule, etc.
discriminatory. in a good sense. ABMs are kinda stupid in choosing when it
should run. FAMs gives more control over when to run or when to idle and at
what stage. idle earlier, the better.
low-level map access. it's really painful to me that i don't have bulk map
access for ABMs. with FAMs we can just iterate tens of nodes quickly without
resorting to get_node(). this should also incentivize some other optimizations
i haven't realize yet.
flexible. with more tweak to this, we could possibly add more mcl2-specific
and generic optimizations and edge cases ABMs failed to fulfill. additionally,
i've written such that the API is more dynamic compared to ABMs.

cons:

easy to prototype. difficult to write a complete, full implementation.
frequency/interval-based callback → a different sledgehammer to abuse.
polllutes globalstep. now our time budget is much smaller → 0.1s. it's very
easy to fuck this up. lag spikes is likely to be introduced.
involves voxelmanip.
there's prob other cons i didn't list, but i'm tired of writing this rn.

### node globalstep → focused area manips i think the current proposed node globalstep is kinda restrictive. we don't always want to presistently run a callback of a single node within range of a player. i think we can generalise this further into a globalstep-based API: focused area manips ([FAMs](https://www.urbandictionary.com/define.php?term=fam)). the idea is that we want to persistantly run updates within an area of a focus point, nearby a player, until (optionally) a condition is reached. ```lua -- related APIs to consider implementing: -- LuaEntity's on_step() but for players mcl_proposal.register_on_stepplayer(function(player, dtime) ... end) --- must perform vector.sort(box[1], box[2]) when constructing these instances --- @type Box {Vector, Vector} -- WARNING: there's an overhead from all these function constructors. -- be sure to cache them if using repeatedly -- WARNING: a read-only LVM is used because FAMs requires low-level, fast bulk map access -- BUT also intended for high-level, unimpaired map write. -- read-only as in, you cannot write to map, but you may use vm:set_*_data(). -- registers a focused area manip (FAM), even during runtime (executes on the next server step) mcl_proposal.register_fam{ -- see ABM definition label = "*pop* noice", chance = 1, -- (default) catch_up = false, -- (default) interval = number, player = function() return true end, -- (default) activates for all players -- activates according to player by name player = mcl_proposal.fam.player_name{'player1', ...}, --- @return boolean whether to activate FAM for player or not (true → activates) player = function(player) ... end, -- dynamically choose players to activate -- (default) player as focus focus = mcl_proposal.fam.focus_player(), -- absolute or relative position as focus -- distance determines maximum distance of absolute position from the player to be considered as a focus focus = mcl_proposal.fam.focus_pos("relative", {Vector, ...}), focus = mcl_proposal.fam.focus_pos("absolute", distance, {Vector, ...}), focus = mcl_proposal.fam.focus_pos({--[[absolute]] distance, Vector, ...}, {--[[relative]] Vector, ...}), -- both --- @return Vector|Vector[]|nil, nil|Box|Box[] --- (1st return) Vector|Vector[] focus position(s) used for calculating areas --- (1st return) nil no focus positions found, skipping --- (2nd return) nil area given by area(player, fpos). see below --- (2nd return) Box|Box[] area(s) given for (each of) focus position(s) focus = function(player) ... end, -- dynamically choose focus positions -- area extrudes from fpos, either with equal radius in all directions or with radius defined by Vector area = mcl_proposal.fam.area_extrude(number|Vector), -- area with equal length in all directions or with length defined by Vector centers at fpos area = mcl_proposal.fam.area_fixed(number|Vector), -- area from player's position to fpos or if given, fpos:add(Vector) area = mcl_proposal.fam.area_from_player([Vector]), --- @param fpos Vector a focus position --- @return Box area to read from map area = function(player, fpos) ... end, -- dynamically choose areas nodenames = nil, -- allow action() to manually search instead nodenames = { 'nodenames' }, -- list of nodes to apply action() neighbors = { 'nodenames' }, -- list of node neighbors. see ABM definition -- see ABM definition (there's no X and Z limits for ABMs for whatevs reason) -- these should be hardcapped to whatever map limits mcl2 already calculated min_x = number, max_x = number, min_y = number, max_y = number, min_z = number, max_z = number, --- action() when nodenames is nil --- @param player ObjectRef --- @param focus Vector position of focus (player, position or node) --- @return nil|number|true --- number sets a temporary interval for next callback --- true will destroy the FAM, allowing for temporary FAMs action = function(player, focus) ... end --- action() when nodenames is a list of nodes --- @param player ObjectRef --- @param focus Vector position of focus (player, position or node) --- @param pos_list Vector[] positions of matching nodes --- @param nodes Nodes[] matching nodes --- @return nil|number|true --- number sets a temporary interval for next callback --- true will destroy the FAM, allowing for temporary FAMs action = function(player, focus, pos_list, nodes) ... end } ``` ```lua -- detailed look at how FAM could be implemented ------------------------------- for each server step: for each player: local playerpos = player:get_pos() local fam = list of FAMs that passed the interval, chance (w catch_up) and fam.player check -- obtain areas local fpos, area, ids = {}, {}, {} local f, a, tf, ta, prev_id for each fam as fam: f, a = fam.focus(player) tf, ta = type(f), type(a) if not f: continue elseif not a and tf ~= "table": ids[fam.id] = (ids[prev_id] or 0)+1 fpos:insert(f) area:insert(fam.area(player, f)) elseif not a: ids[fam.id] = (ids[prev_id] or 0)+#f for each f: fpos:insert(each f) area:insert(fam.area(player, each f)) elseif tf ~= ta: error: types of both returns from fam.focus() must be the same elseif tf == 'table' and #f ~= #a: error: length of both returns from fam.focus() must be equal else: fpos:add(f) area:add(a) ids[fam.id] = (ids[prev_id] or 0)+#f prev_id = fam.id end -- list of FAMs that passed the focus check (fam.focus() 1st return is non-nil) fam = fam:filter(fam -> ids[fam.id]) -- access the map -- TODO: solve this math problem local boxes = calculate list of boxes to read from map, striking a balance between volume outside areas (wasted/unused data) and volume of total boxes local boxes_area = list with index corresponding to an area. each element is an index for boxes corresponding to a box containing an area -- RESEARCH: make a graph of time took to read from map and cube side length using methods: minetest.get_node(), LVM -- TODO: fallback to minetest.get_node() under areas with volume smaller than X? local voxelmanips, cids, params, param2s = {}, {}, {}, {}, {} for each boxes as b: local vm = ReadOnlyVoxelManip(b[1], b[2]) cids:insert(vm:get_data()) params:insert(vm:get_light_data()) param2s:insert(vm:get_param2_data()) -- execute the active FAMs -- pun: vi? no i use emax -- reused variables: fpos, area, ids local ib, ie = 1, nil local destroy_ids = {} local status for each fam as fam: ie = ids[fam.id] for each (fpos:sub(ib, ie)) as f, each (area:sub(ib, ie)) as a, each (boxes_area:sub(ib, ie)) as each bi, each (boxes[each bi]) as box, each (VoxelArea(box[1], box[2])) as va, each (voxelManips[each bi]) as vm, each (cids[each bi]) as cid_data, each (params[each bi]) as param_data, each (param2s[each bi]) as param2_data: -- fam.node_cids → mapping of searched node content IDs to their nodename, set when registering FAM -- fam.neighbor_cids → mapping of neighbor content IDs to their nodename, set when registering FAM if fam.node_cids and fam.neighbor_cids: -- search with neighbors local pos_list, node_list = {}, {} for each (range(box[1].z, box[2].z)) as z: for each (range(box[1].y, box[2].y)) as y, for each (range(box[1].x, box[2].x)) as x, each (va:index(x, y, z)) as vi: local name = fam.node_cids[cid_data[vi]] if name and fam.neighbor_cids[cid_data[va:index(any 6 adjacent nodes)]]: pos_list:insert(vector.new(x, y, z)) node_list:insert({node=name, param=fam.param_data[vi], param2=fam.param2_data[vi]}) status = apply(player, f, pos_list, node) elseif fam.node_cids: -- search without neighbors local pos_list, node_list = {}, {} for each (range(box[1].z, box[2].z)) as z: for each (range(box[1].y, box[2].y)) as y, for each (range(box[1].x, box[2].x)) as x, each (va:index(x, y, z)) as vi: local name = fam.node_cids[cid_data[vi]] if name: pos_list:insert(vector.new(x, y, z)) node_list:insert({node=name, param=fam.param_data[vi], param2=fam.param2_data[vi]}) status = apply(player, f, pos_list, node) else: -- manual search status = apply(player, f, a, b, va, vm, cid_data, param_data, param2_data) if type(status) == "number": fam.tinterval = status -- temporary interval elseif status: fam.destroy_ids:insert(fam.id) -- destroy this FAM on next server step else: fam.tinterval = nil ib = ie+1 ``` **pros**: - relatively easy to use. if you're familiar with ABMs or LBMs, this should be very familiar. - persistent. ABMs starts dropping whatever it wants if the runtime is too expensive. FAMs should give more control to us to prioritize, schedule, etc. - discriminatory. in a good sense. ABMs are kinda stupid in choosing when it should run. FAMs gives more control over when to run or when to idle and at what stage. idle earlier, the better. - low-level map access. it's really painful to me that i don't have bulk map access for ABMs. with FAMs we can just iterate tens of nodes quickly without resorting to get_node(). this should also incentivize some other optimizations i haven't realize yet. - flexible. with more tweak to this, we could possibly add more mcl2-specific and generic optimizations and edge cases ABMs failed to fulfill. additionally, i've written such that the API is more dynamic compared to ABMs. **cons**: - easy to prototype. difficult to write a complete, full implementation. - frequency/interval-based callback → a different sledgehammer to abuse. - polllutes globalstep. now our time budget is much smaller → 0.1s. it's **very easy** to fuck this up. lag spikes is likely to be introduced. - involves voxelmanip. - there's prob other cons i didn't list, but i'm tired of writing this rn.

Ghost commented

2022-08-16 10:21:08 +02:00

relevant images from ruben about map read/write (lower is better):

relevant images from ruben about map read/write (lower is better): ![image](/attachments/76dadd84-6515-4a49-9491-45ea8553521e) ![image](/attachments/b4e0d61c-6eb0-45f0-bd63-6ed420f8ea04)

image.png

45 KiB

image.png

41 KiB

Michieal referenced this issue

2023-02-04 10:04:56 +01:00

Sculk code needs to be more performant #3394

Michieal referenced this issue

2023-02-04 10:05:30 +01:00

Kelp is quite expensive #3395

ancientmarinerdev added the

performance

#P3: elevated

labels 2023-02-04 15:04:46 +01:00

kneekoo referenced this issue

2023-02-04 22:10:36 +01:00

Sculk code needs to be more performant #3394

ancientmarinerdev added

#P4 priority: medium

and removed

#P3: elevated

labels 2023-03-26 00:57:48 +01:00

teknomunk commented

2024-05-03 12:28:20 +02:00

Here's my proposal to throw onto this pile:

Bulk Node Steps (BNS)

Oxidation Example (ABM Replacement):

vl_bulk_nodestep.register({
	label = "Oxidize Nodes",
	act = function(block, dtime)
		local positions = block:select(dtime, {
			nodenames = {"group:oxidizable"},
			interval = 500,
			chance = 3
		})
		
		for i=1,#positions do
			local pos = positions[i]
			local node = block:get_node(pos)
			local def = minetest.registered_nodes[node.name]
			if def and def._mcl_oxidized_variant then
				node.name = def._mcl_oxidized_variant
				block:swap_node(pos, node)
			end
		end
	end
})

Furnace Example (Node Timer Replacement):

function furnace_node_timer(pos, dtime, block)
	local meta = block:get_meta(pos)
	
	-- Process furnace
	
	if active then
		-- ...
	else
		block:swap_node(pos, "mcl_furnaces:furnace")
		block:get_node_timer(pos):stop()
	end
end

vl_bulk_nodestep.register({
	label = "Update Furnaces",
	needs_metadata = {"mcl_furnaces:furnace", "mcl_furnace:furnace_active"},
	act = function(block, dtime)
		local positions,dtimes = block:select_due_timers({
			nodenames = {"mcl_furnaces:furnace","mcl_furnaces:furnace_active"}
		})
		
		for i=1,#positions do
			furnace_node_timer(positions[i], dtimes[i], block)
		end
	end
})

Pros:

Processes active blocks as 16x16x16 map blocks using VoxelManip
Variable dtime applied to all nodes in a block, to allow optimizing over larger timesteps
May be possible to use Async Environment to pull processing completely out of the main thread
Full control over when updates run and which map blocks are updated
Single facility to replace both active block modifiers and node timers

Cons:

Complex
- Using VoxelManip will require implementing a custom swap_node and adding code to run on_construct/on_destroy
- Async processing will add additional complexity
- Will need to track active blocks ourselves
Different enough from both active block modifiers and node timers to require significant rework of existing code to utilize
Probably others that I haven't thought of yet

Here's my proposal to throw onto this pile: Bulk Node Steps (BNS) Oxidation Example (ABM Replacement): ````lua vl_bulk_nodestep.register({ label = "Oxidize Nodes", act = function(block, dtime) local positions = block:select(dtime, { nodenames = {"group:oxidizable"}, interval = 500, chance = 3 }) for i=1,#positions do local pos = positions[i] local node = block:get_node(pos) local def = minetest.registered_nodes[node.name] if def and def._mcl_oxidized_variant then node.name = def._mcl_oxidized_variant block:swap_node(pos, node) end end end }) ```` Furnace Example (Node Timer Replacement): ````lua function furnace_node_timer(pos, dtime, block) local meta = block:get_meta(pos) -- Process furnace if active then -- ... else block:swap_node(pos, "mcl_furnaces:furnace") block:get_node_timer(pos):stop() end end vl_bulk_nodestep.register({ label = "Update Furnaces", needs_metadata = {"mcl_furnaces:furnace", "mcl_furnace:furnace_active"}, act = function(block, dtime) local positions,dtimes = block:select_due_timers({ nodenames = {"mcl_furnaces:furnace","mcl_furnaces:furnace_active"} }) for i=1,#positions do furnace_node_timer(positions[i], dtimes[i], block) end end }) ```` Pros: * Processes active blocks as 16x16x16 map blocks using VoxelManip * Variable dtime applied to all nodes in a block, to allow optimizing over larger timesteps * May be possible to use Async Environment to pull processing completely out of the main thread * Full control over when updates run and which map blocks are updated * Single facility to replace both active block modifiers and node timers Cons: * Complex * Using VoxelManip will require implementing a custom swap_node and adding code to run on_construct/on_destroy * Async processing will add additional complexity * Will need to track active blocks ourselves * Different enough from both active block modifiers and node timers to require significant rework of existing code to utilize * Probably others that I haven't thought of yet

teknomunk referenced this issue

2024-05-31 13:24:08 +02:00

WIP: Dynamic Rules and Settings #4347

teknomunk referenced this issue

2024-06-08 19:13:39 +02:00

Fire in distance never extinguishes #4417

ancientmarinerdev commented

2024-06-08 22:10:19 +02:00

I would caution against use of timers. I recently unpicked it from kelp. Basically it used 20% of the CPU because it avoided ABM's. The code was complex, messy, and worse performing than the abm's it was allegedly trying to optimise.

Profiling data here for kelp: #3417

I would caution against use of timers. I recently unpicked it from kelp. Basically it used 20% of the CPU because it avoided ABM's. The code was complex, messy, and worse performing than the abm's it was allegedly trying to optimise. Profiling data here for kelp: https://git.minetest.land/VoxeLibre/VoxeLibre/pulls/3417

👍 1

ancientmarinerdev referenced this issue

2024-06-08 22:14:12 +02:00

Fire in distance never extinguishes #4417

teknomunk commented

2024-06-08 22:15:39 +02:00

I would caution against use of timers. I recently unpicked it from kelp. Basically it used 20% of the CPU because it avoided ABM's. The code was complex, messy, and worse performing than the abm's it was allegedly trying to optimise.

I agree. Node timers would be a terrible idea.

I haven't even started making a prototype of my proposal, but my first though on how to implement it is using a global step or a periodic scheduler task that selectively runs updates on map blocks around players.

A facility like what is described here might be useful in completely eliminating the use of node timers. Provided, of course, that it doesn't have its own performance issues.

> I would caution against use of timers. I recently unpicked it from kelp. Basically it used 20% of the CPU because it avoided ABM's. The code was complex, messy, and worse performing than the abm's it was allegedly trying to optimise. I agree. Node timers would be a terrible idea. I haven't even started making a prototype of my proposal, but my first though on how to implement it is using a global step or a periodic scheduler task that selectively runs updates on map blocks around players. A facility like what is described here might be useful in completely eliminating the use of node timers. Provided, of course, that it doesn't have its own performance issues.

👍 1

ancientmarinerdev commented

2024-06-08 22:47:33 +02:00

On the topic of ABM's though, if we want to improve things, we could potentially make them more frequent with less chance. So if for example, it runs on every node every 10s, making it run on 1/10th of the nodes every second, or 1/20th of the nodes every 0.5s could spread that work out evenly and cut down on lag spikes, or ticks in which ABM's have to abandon from too much processing happening in that tick. Could be worth doing a little analysis on this first for the worst offenders.

It would be great to hear people's views around this.

On the topic of ABM's though, if we want to improve things, we could potentially make them more frequent with less chance. So if for example, it runs on every node every 10s, making it run on 1/10th of the nodes every second, or 1/20th of the nodes every 0.5s could spread that work out evenly and cut down on lag spikes, or ticks in which ABM's have to abandon from too much processing happening in that tick. Could be worth doing a little analysis on this first for the worst offenders. It would be great to hear people's views around this.

teknomunk commented

2024-06-08 22:59:07 +02:00

I think spreading out the load across more time steps could be a good way to reduce lag spikes. I do have some mild concern about making sure that nodes don't get starved of updates. I'm not sure how minetest handles its chance and depending on implementation, some nodes could be processed more, and others less, than an ABM that has a chance=1 would do.

Could be worth doing a little analysis on this first for the worst offenders.

Absolutely worth it in my opinion. First step to resolving performance issues is to get reliable data on where the code is spending its time. Just a matter of prioritizing available developer time, is all.

I think spreading out the load across more time steps could be a good way to reduce lag spikes. I do have some mild concern about making sure that nodes don't get starved of updates. I'm not sure how minetest handles its chance and depending on implementation, some nodes could be processed more, and others less, than an ABM that has a chance=1 would do. > Could be worth doing a little analysis on this first for the worst offenders. Absolutely worth it in my opinion. First step to resolving performance issues is to get reliable data on where the code is spending its time. Just a matter of prioritizing available developer time, is all.

👍 1

WillConker referenced this issue

2024-07-09 15:52:33 +02:00

WIP: Dynamic Rules and Settings #4347

teknomunk commented

2024-09-01 22:11:49 +02:00

Some more information about the solution that Exile uses:

kromka-chleba said:

@lhofhansl, I'm the author of the crazy workaround made for Exile.

Some problems I encountered with ABMs:

Weak performance, if you add too much the game becomes laggy or ABMs don't do the job they should

Limited range to active block radius (which could be seen as a feature or a limitation depending on task)

Pitiful control over execution: can only register once, can't unregister or change, the only control over their execution is chance and interval. This also makes them static and inflexible

The above makes ABMs a poor choice for not-so-abstract tasks such as snow/ice placing, soaking soil with water, changing seasons. I tried using ABMs and LBMs for these tasks in Exile, they delivered poor outcomes, e.g. changing seasonal soil, placing snow, etc. only in the active block radius around players with big intervals to prevent massive lags.

To solve these problems I was forced to write a mod called Mapchunk Shepherd (that is terribly written and poorly documented btw, will fix this soon-ish) which is basically a system for assigning labels to mapchunks and then processing these mapchunks with Voxel Manip. The advantage of my system is that worker functions run only when they're needed (unlike ABMs) so ice freezers work only when temperature falls below zero, snow is only placed when it snows, seasonal soil and plants are only replaced in mapchunks with off-season flora. The advantage of my system is also that it works on all loaded mapchunks (mapblocks technically) so these tasks are performed up to the horizon with reasonable performance.

Some more information about the solution that Exile uses: kromka-chleba [said](https://github.com/minetest/minetest/issues/15092#issuecomment-2323469242): >@lhofhansl, I'm the author of the crazy workaround made for Exile. > > Some problems I encountered with ABMs: > > 1. Weak performance, if you add too much the game becomes laggy or ABMs don't do the job they should > 2. Limited range to active block radius (which could be seen as a feature or a limitation depending on task) > 3. Pitiful control over execution: can only register once, can't unregister or change, the only control over their execution is chance and interval. This also makes them static and inflexible > > The above makes ABMs a poor choice for not-so-abstract tasks such as snow/ice placing, soaking soil with water, changing seasons. I tried using ABMs and LBMs for these tasks in Exile, they delivered poor outcomes, e.g. changing seasonal soil, placing snow, etc. only in the active block radius around players with big intervals to prevent massive lags. > > To solve these problems I was forced to write a mod called [Mapchunk Shepherd](https://codeberg.org/perfect_city_game_studio/mapchunk_shepherd) (that is terribly written and poorly documented btw, will fix this soon-ish) which is basically a system for assigning labels to mapchunks and then processing these mapchunks with Voxel Manip. The advantage of my system is that worker functions run only when they're needed (unlike ABMs) so ice freezers work only when temperature falls below zero, snow is only placed when it snows, seasonal soil and plants are only replaced in mapchunks with off-season flora. The advantage of my system is also that it works on all loaded mapchunks (mapblocks technically) so these tasks are performed up to the horizon with reasonable performance.

teknomunk referenced this issue

2024-09-28 16:39:54 +02:00

WIP: Dynamic Rules and Settings #4662

Sign in to join this conversation.