Batching APIs

[SoniEx2]

Things such as minetest.register_craft could be batched (e.g. minetest.batch.register_craft()).

Instead of taking a single recipe, it'd take a table of recipes. e.g.

minetest.batch.register_craft({
{
    output = "default:chest",
    recipe = {
        {"default:wood", "default:wood", "default:wood"},
        {"default:wood", "", "default:wood"},
        {"default:wood", "default:wood", "default:wood"}
    }
},
{
    type = "shapeless",
    output = "wool:green",
    recipe = {"group:wool", "dye:green"},
    replacements = {
        {"dye:green", "test:testnode"}
    }
}
})

For multi-argument functions, it'd take a table for each argument.

For return values, it'd return a table for each return. e.g.

local list_of_output, list_of_decremented_input = minetest.batch.get_craft_result({
{ method = "normal", width = 1, items = { ItemStack("default:coal_lump 2"), ItemStack("default:stick 2") }}
})
local output, decremented_input = list_of_output[1], list_of_decremented_input[1]
-- output.item:to_table() = {wear = 0, name = "default:torch", count = 4, metadata = ""}
-- output.time = 0
-- decremented_input.method = "normal"
-- decremented_input.width = 1
-- #decremented_input.items = 2
-- decremented_input.items[1]:to_table() = {wear = 0, name = "default:coal_lump", count = 1, metadata = ""}
-- decremented_input.items[2]:to_table() = {wear = 0, name = "default:stick", count = 1, metadata = ""}

[kwolekr]

Why?

[SoniEx2]

For things such as adding recipes, it's supposed to be faster when adding procedurally generated stuff.

For things such as crafting, it's supposed to be faster when crafting multiple recipes at once. (megafurnace that can be smelting 10 different items at a time, all in parallel?)

Mainly a LuaJIT optimization. (e.g. procedural generation + recipe registration is a loop, vs procedural generation into a table, then a single Lua/C API call to register all recipes)

[RobertZenz]

What is your exact use case? I'm not seeing how this would improve the API, but maybe I'm missing something.

As example, instead of

minetest.register_craft(definition)
minetest.register_craft(definition)
minetest.register_craft(definition)

you'd write with this batch API

minetest.batch.register_craft(
    definition,
    definition,
    definition)

That doesn't really add or remove anything. It neither does remove lines of code, nor does it add something to the readability. Quite the opposite actually regarding your your suggestion regarding multiple arguments.

minetest.do_something(valueA1, valueB1, valueC1)
minetest.do_something(valueA2, valueB2, valueC2)
minetest.do_something(valueA3, valueB3, valueC3)

Would turn into

minetest.do_something({
    valueA1, valueA2, valueA3
}, {
    valueA1, valueA2, valueA3
}, {
    valueA1, valueA2, valueA3
})

which makes it harder to find the values that belong to each other.

I can see where you want to go if you mean an optimization, removing and reducing function calls can indeed lead to better performance, but this is maybe giving better performance with a very hard downside. Also, register_craft might be called often, but only once when the game/server is started, so it is in my opinion neglectable unless you register several tens of thousands of items and nodes (and I doubt that under the 100k threshold it is worth talking about optimizations).

Additionally most procedural generation is happening inside loops, so you're swapping function calls against memory/appending to a table to gather first all values and then insert them at one swoop.

This might actually be needed at some areas, but the handling of crafts isn't one of them.

[red-001]

This wouldn't improve performance as craft recipes are registered on server load and it wound decrease readability a lot.

[est31]

Heh lol I guess we would trade the speed benefit (if there is any) in thanks to the additional table lookup we do with .batch.

I don't think this is needed, startup is pretty fast.

[est31]

For me, it takes 1.9 seconds to register 20k craft recipes with the test I wrote for #2210. That's 0.5 ms per recipe. Can you implement such a batch call, and benchmark that?

[SoniEx2]

@RobertZenz @red-001
Example: (based on this)

local list = {1, 5, 10, 25, 50, 100, 200, 500, 1000, 2500, 5000, 10000}
local function registerRecipesRecursive(left, index, recipe, id)
  if left > 0 then
    if index > 9 then return end -- max 9 items
    for x, v in ipairs(list) do
      -- check if the item is suitable (avoid duplicated recipes)
      if (index == 1 or v >= list[recipe[index - 1]]) and v <= left then
        local newRecipe = {}
        for k,v in pairs(recipe) do
          newRecipe[k] = v
        end
        newRecipe[index] = x
        registerRecipesRecursive(left - v, index + 1, newRecipe, id)
      end
    end
  elseif left == 0 then
    local recipeData = {}
    for i,v in ipairs(recipe) do
      recipeData[i] = "money:money " .. v
    end
    minetest.register_craft({
        type = "shapeless",
        output = "money:money " .. id,
        recipe = recipeData,
      })
  end
end
local function registerRecipes()
  for i=1,#list do
    registerRecipesRecursive(list[i], 1, {}, i)
  end
end

vs

local list = {1, 5, 10, 25, 50, 100, 200, 500, 1000, 2500, 5000, 10000}
local function registerRecipesRecursive(left, index, recipe, id, result)
  if left > 0 then
    if index > 9 then return end -- max 9 items
    for x, v in ipairs(list) do
      -- check if the item is suitable (avoid duplicated recipes)
      if (index == 1 or v >= list[recipe[index - 1]]) and v <= left then
        local newRecipe = {}
        for k,v in pairs(recipe) do
          newRecipe[k] = v
        end
        newRecipe[index] = x
        registerRecipesRecursive(left - v, index + 1, newRecipe, id, result)
      end
    end
  elseif left == 0 then
    local recipeData = {}
    for i,v in ipairs(recipe) do
      recipeData[i] = "money:money " .. v
    end
    table.insert(result, {
        type = "shapeless",
        output = "money:money " .. id,
        recipe = recipeData,
      })
  end
end
local function registerRecipes()
  local result = {}
  for i=1,#list do
    registerRecipesRecursive(list[i], 1, {}, i, result)
  end
  minetest.batch.register_craft(result)
end

Really doesn't change much when it comes to readability.

It shouldn't be varargs because you can't table.insert into varargs (altho varargs would potentially be faster), and you'd have issues with multiple arguments if one (or more) of the arguments were optional.

[est31]

Some people say table.insert is slow?

[SoniEx2]

@est31 Not in LuaJIT. In LuaJIT table.insert is actually faster than keeping track of the count/size yourself.

[RobertZenz]

@SoniEx2 It's a little bit late on my end, so excuse me if I'm wrong, but am I seeing this right that this function registers 12 "coin sizes" and (most?) possible receipts for that?

[SoniEx2]

@RobertZenz All possible 3x3 recipes (e.g. it won't try to do a 10000 x "coin:coin 1" recipe), yes.

[RobertZenz]

I know that this is off-topic, but why not simply let the coins stack until 10,000? You only have one coin, no different sizes, no special cases, just damn big stacks.

[PilzAdam]

I doubt that this will result in a useful performance increase.

[SoniEx2]

If not directly result in a performance increase, it will, at least, encourage batching.

Alternatively add a "modding best practices" wiki page and hope people read it...

[PilzAdam]

@SoniEx2 what would be the benefit of this? It seems you only want to encourage a different style.

[SoniEx2]

The benefits are to:

1. Avoid hot loops that can't be JITted
2. Uhh... Only 1, really...

... I guess a "modding best practices" wiki page would be a better idea... (should include things such as "avoid loadstring, pass functions around instead")

[est31]

To test whether batching in fact increases speed, as you claim, I've implemented what you request, together with a benchmark: https://github.com/est31/minetest/commits/batch_api_test

To run the test, just get the code, and start a world with the minimal game. It will test-wise register 20k recipes, and print the benchmark results to stdout.

Using LuaJIT, it gives me output:

time spent registering using batched method: 1624739 us (per call: 81.23695 us)
time spent registering using traditional method: 1659139 us (per call: 82.95695 us)
time advantage of batched method: 34400 us (per call: 1.72 us)

Using bundled lua, it gives me:

time spent registering using batched method: 1784569 us (per call: 89.22845 us)
time spent registering using traditional method: 1830328 us (per call: 91.5164 us)
time advantage of batched method: 45759 us (per call: 2.28795 us)

You are right, there is an advantage of using the batched method, but the advantage is so small (2% to 4%), that I don't think we should add it. Therefore, closing.

[SoniEx2]

What if you use procedural generation (e.g. the code I provided)?

[est31]

I don't think it would be much different. If you want, you can try it, but I doubt your results will be different from mine.

[SoniEx2]

Also was table.insert in plain Lua actually faster than the traditional method?! I wasn't expecting that one... O_o