minetest-mods / craftguide

:book: The most comprehensive Crafting Guide on Minetest
Other
43 stars 20 forks source link

Cache items by group ans speed up group-related operations #51

Closed bell07 closed 5 years ago

bell07 commented 5 years ago

This change does cache all group -> items relations at start in lua table and speed up all recipes operations. This gives performance boost for progressive mode. The function craftguide.get_group_items(str) I exposed to get access to this cache from outside the craftguide. I plan to use them in my progressive-revealed mod

kilbith commented 5 years ago

Tested and benchmarked with dreambuilder with 15 items in inventory. Speed is almost similar actually...

Before:

Speed: 248.00 ms
Speed: 340.00 ms
Speed: 199.00 ms
Speed: 238.00 ms
Speed: 334.00 ms

After:

Speed: 247.00 ms
Speed: 331.00 ms
Speed: 194.00 ms
Speed: 215.00 ms
Speed: 322.00 ms
p-ouellette commented 5 years ago

I think this is needlessly complicated. The group_stereotypes table is already used as a cache, just add more groupstring-->item mappings to it whenever a new one is resolved in groups_to_item. get_group_items and group_items are not needed (we only need one item, that matches the groupstring, not all of them).

Also, I suggest using Luacheck. It reports several bugs.

bell07 commented 5 years ago

@kilbith, how did you the benchmark? I thestet with the whynot mods collection (new version published yet) and feels the difference, but do not have any benchmark values.

@pauloue, the group_stereotypes are just for get an representative item for a group to be shown in UI. This cache is for faster resolving groups like group:flower,color_red to get all possible items to faster usages calculation and to faster progressive calculcations.

kilbith commented 5 years ago

If you can't benchmark your code, this is not serious. I don't care about feelings, I care about data.

bell07 commented 5 years ago

Hm, you are right, my feel was outdated. I was convinced that new hash table should get the performance boost so I did not a benchmark :-/

Now I benchmarked get_progressive_items() and get_item_usages() Result is sobering, there is no boost, just a small improvement which does not justify a new big cache table :-( Maybe my implementation needs more optimization I do not see currently?

Upstream version:

- Full inventory - progressive
bench get_progressive_items 0.307002
bench get_progressive_items 0.247309
bench get_progressive_items 0.28344899999999
bench get_progressive_items 0.34322
bench get_progressive_items 0.34267699999998
bench get_item_usages   0.14338799999996
bench get_item_usages   0.18894
bench get_item_usages   0.19577900000002
bench get_item_usages   0.13819899999999
bench get_item_usages   0.14081799999997
bench get_item_usages   0.17968300000001
bench get_item_usages   0.13141199999995
bench get_item_usages   0.150793
bench get_item_usages   0.137565
bench get_item_usages   0.13419300000001

- Empty inventory - progressive
bench get_progressive_items 0.22190700000002
bench get_progressive_items 0.32030499999999
bench get_progressive_items 0.455297
bench get_progressive_items 0.20180400000001
bench get_progressive_items 0.31795599999998

- Prorgessive disabled
bench get_item_usages   0.21247499999998
bench get_item_usages   0.22117500000002
bench get_item_usages   0.28052400000001
bench get_item_usages   0.20388500000001
bench get_item_usages   0.28032300000001

My version

- Full inventory - progressive
bench get_progressive_items 0.24002900000005
bench get_progressive_items 0.24308499999995
bench get_progressive_items 0.33904299999995
bench get_progressive_items 0.34184299999993
bench get_progressive_items 0.23163699999998
bench get_progressive_items 0.34269600000005
bench get_item_usages   0.17935799999998
bench get_item_usages   0.22512600000005
bench get_item_usages   0.16827799999999
bench get_item_usages   0.14325300000007
bench get_item_usages   0.15673500000003
bench get_item_usages   0.17923600000006

- Empty inventory - progressive
bench get_progressive_items 0.19492300000002
bench get_progressive_items 0.23205099999996
bench get_progressive_items 0.19488100000001
bench get_progressive_items 0.36649299999999
bench get_progressive_items 0.16545399999995

- Prorgessive disabled
bench get_item_usages   0.127344
bench get_item_usages   0.134959
bench get_item_usages   0.143638
bench get_item_usages   0.135097
bench get_item_usages   0.13208
bench get_item_usages   0.19215000000001
bench get_item_usages   0.128412
bench get_item_usages   0.141422
bench get_item_usages   0.209862
bell07 commented 5 years ago

I admit this is the second valueless PR in order from my site. Since the caching did not adduce the expected performance boost, this PR can be closed without merge.

Sorry for the inconvenience.