minetest-mods / unified_inventory

An extensible inventory mod which allows searching crafting and browsing for recipes in the same dialogue.
Other
50 stars 38 forks source link

Performance Boost #160

Closed ghost closed 3 years ago

ghost commented 3 years ago

I was comparing inventory mods for features and performance when I noticed something interesting. The idle CPU usage when the Unified Inventory was opened but not doing anything made Minetest use around 16% CPU on my computer. However, Crafting Guide (https://github.com/minetest-mods/craftguide) in the same Minetest world only made Minetest use around 8%. Both tests simply involved opening the menu and doing nothing. Both show a similar amount of items on the screen. How could this be that Crafting Guide uses so much less cpu?

The only thing I noticed was different was the styling. Unified Inventory shows the button background/boarder on the items while Crafting Guide does not. So I added on line 174 in register.lua "style_type[item_image_button;border=false]" to the formspec variable. Not sure if that's the proper place to put it, but it works for the test. I tested Unified Inventory again and sure enough, the idle CPU was now around 12%. While that is still higher than the Crafting Guide mod, that's a pretty big difference. I'm not sure why, but it seems having the button background/border causes a lot more work for Minetest.

I'm hoping someone else can test this on their computer and verify if and how much CPU usage is lowered. It's possible this might not make as much of a difference on different hardware. If it lowers the CPU usage for other people, it might be worth changing.

SmallJoker commented 3 years ago

item_image_button or similar is very slow to render, because it's AFAIK done on CPU. Engine issue.

ghost commented 3 years ago

I figured it was an engine issue, but I wanted to mention it. Should I open an issue on the Minetest project about it or is this a known issue?

Also, if the issue is expected to be fixed soon or if this project is not interested in the changes, you're welcome to close this.

SmallJoker commented 3 years ago

item_image_button already exists for a long time, and speed optimizations only happen if someone is really bothered by them. As long it works, it's usually good enough. The simplest solution right now would be to enable the slim/small mode of unified_inventory to reduce the rendered button count.

ghost commented 3 years ago

With slim mode enabled, there is still a difference of about 2% CPU usage when disabling the item button backgrounds. Most won't care about that, so it probably doesn't matter to change it. Since I don't really care for the backgrounds anyway, I'll just implement the changes on my own copy.

kilbith commented 3 years ago

craftguide renders more complex stuff on the screen than unified_inventory and still manages to be much cheaper on the CPU usage.

Simply put: better code.

@isaiah658 Give a try to https://github.com/minetest-mods/i3

daytonnvpaul commented 3 years ago

@kilbith I follow this repo's notifications because I use this mod, and to be made aware of issues and updates. This certainly isn't the place to promote your mod.

Edit for grammar.