Skip to content
This repository has been archived by the owner on Aug 9, 2023. It is now read-only.

Refactors, performance improvements, and fixes #54

Closed
wants to merge 12 commits into from
Closed

Refactors, performance improvements, and fixes #54

wants to merge 12 commits into from

Conversation

p-ouellette
Copy link
Contributor

Benchmark in dreambuilder:

master:

[craftguide] Getting init_items
[craftguide] Found 1722 init_items (16592 reg_items) in 16.0126 sec
[craftguide] Running benchmark
get_item_usages
  11.6449
  10.9920
  13.0139
  16.5721
  12.1848
  11.5292
  12.1791
  16.7743
  10.9867
  11.3969
  10.8906
  12.3318
  16.0794
  11.1595
  11.8558
  avg: 12.6394 ms
get_progressive_items
  21.6717
  18.7699
  22.8427
  23.1046
  29.9226
  17.1253
  17.7711
  26.0294
  24.1830
  32.0850
  17.3702
  17.9929
  26.0571
  22.5991
  30.8307
  avg: 23.2237 ms

PR:

[craftguide] Getting init_items
[craftguide] Found 1722 init_items (16592 reg_items) in 16.2989 sec
[craftguide] Running benchmark
get_item_usages
  0.9279
  0.8491
  0.8352
  1.0106
  1.1313
  1.1292
  1.1588
  1.0636
  0.8733
  1.1002
  1.0572
  1.0998
  1.1199
  1.1237
  0.8954
  avg: 1.0250 ms
get_progressive_items
  7.2115
  12.0551
  8.6089
  7.4014
  8.0327
  19.7317
  8.1721
  7.8774
  28.4355
  19.8928
  6.6494
  6.1713
  6.2526
  6.8371
  6.5223
  avg: 10.6568 ms

There's no significant difference in the performance of get_init_items, get_item_usages("default:stick") is ~92% faster, and get_progressive_items is ~54% faster.

To test, the benchmark commit can be cherry-picked from my master branch.
Fuels are cached because get_craft_result is very slow (try commenting out cache_fuel in get_init_items).

Also fixes shapeless icon not showing and the wrong tooltip being shown before that.
Sorry it's so big.

init.lua Outdated
@@ -986,3 +933,36 @@ for x = 1, 6 do
})
end
]]

minetest.register_on_mods_loaded(function()
Copy link
Member

@SmallJoker SmallJoker Jan 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause an error on the latest stable build (0.4.17.1), since it's a 5.0.0-dev feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

register_on_mods_loaded is already used elsewhere and the currently released version is 5.0-only. Also this part will probably be dropped as it's just for testing.

end

local function get_recipes(output)
local function cache_recipes(output)
local recipes = mt.get_all_craft_recipes(output) or {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you make use of already existing entries in recipes_cache to skip this lookup?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the function that adds an item's recipes to the cache and it's only called once per item in get_init_items.

init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
init.lua Outdated Show resolved Hide resolved
@kilbith
Copy link
Collaborator

kilbith commented Jan 22, 2019

Merged, thanks.

d950c71

@kilbith kilbith closed this Jan 22, 2019
@p-ouellette p-ouellette deleted the refactor branch January 22, 2019 01:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants