search

minetest-mods / craftguide

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

get_filtered_items fix #67

Closed p-ouellette closed 5 years ago

p-ouellette commented 5 years ago

I didn't benchmark anything but I can't detect any difference in performance between this commit, https://github.com/minetest-mods/craftguide/commit/fc2d2e585c464fe1892cb778c0bd83b04deae2dd and https://github.com/minetest-mods/craftguide/commit/4560457504a377fc84f5435a38fcc99eb87e4372. Maybe the performance regression you noticed was due to https://github.com/minetest-mods/craftguide/commit/c1b1bef263f8f8889e1303b0678f17ec263d942b? That commit does make a pretty big difference for me.

get_filtered_items calls apply_recipe_filters for each of the init_items and the progressive filter updates the discovered items. So the discovered items get updated hundreds of times every time the guide is opened. Updating discovered items outside of the filter should fix this. Maybe a globalstep, but updating all players at the same time could be problematic.

At e6268a395f7309a2bd16fd3ad8f59fa92b202ad4 the guide opens instantly and at c1b1bef263f8f8889e1303b0678f17ec263d942b the delay is very noticable.

kilbith commented 5 years ago

On my tests (w/ LuaJIT), get_filtered_items is ~250% slower since https://github.com/minetest-mods/craftguide/commit/fc2d2e585c464fe1892cb778c0bd83b04deae2dd.

It's even worse with your changes, ~350% slower (up to 130+ ms).

p-ouellette commented 5 years ago

This might be faster if updating discovered items gets moved out of the progressive filter. Doing that would make a much bigger difference in any case.

kilbith commented 5 years ago

Go for it then.

kilbith commented 5 years ago

https://github.com/minetest-mods/craftguide/commit/21b312e5cb452c6330b96e1ff73c4b249838dab8