Remove some unneeded memory allocation #614
Conversation
The function was supposed to return an outputrule, but before it inverted the returned rules, so these usages of the function expected inverted rules.
mesecons/internal.lua
Outdated
| local function get_rules(def, node) | ||
| local rules = def.rules | ||
| if type(rules) == 'function' then | ||
| if not def.const_node then |
There was a problem hiding this comment.
Do you happen to know where this would be documented?
Also: How much of a performance difference are we talking about here? A new variable and checks to skip a table creation with three indices seems to be overkill - at least to me.
There was a problem hiding this comment.
I'm not sure where this should be documented. I can't tell the source of the documentation at https://mesecons.net/developers.html.
I think avoiding copying is worth it, since the garbage collector is a known limitation to the speed of LuaJIT. If you have 10k insulated wire segments, that's 10k allocations saved.
There was a problem hiding this comment.
I believe that code complexity (O) matters most. If the function calls are proportional to the amount of placed wires, such an optimization does not seem to be urgent due to "sane" linear scaling in computational. Of course it does make a small difference, but nothing compared to the case if it would scale by O(n²). Hence the time used by an allocation and later GC appears negligible.
Either way ... what would you think of renaming the variable to rule_node_nocopy? action_on still receives a copy after all.
There was a problem hiding this comment.
Optimizations to the code complexity have the highest yield, but eventually the only optimizations left to do involve reducing the constant factor. I think these are still important since people use Mesecons to build vast contraptions such as whole computers. In these cases, even a 1% optimization reduces the absolute execution time by a lot.
I will rename the variable to rule_node_nocopy.
|
Perhaps someone with the right privilege can run a search of ContentDB for mods using |
|
Thanks. It looks like lwwires is the only one. |
|
I decided to reimplement the functions regardless. |
|
By the way, this code could be simplified and sped up a bit if it were assumed that the functions which return rules do not modify the node tables passed to them. This assumption is not the norm in the Minetest API, but it seems realistically true. |
Overview
This PR does three things:
mesecon.get_conductor.rule_node_nocopykey in their definitions.Benchmarks
I used the benchmarks from #578. Keep in mind that reducing garbage collection load may also increase the performance of code outside mesecons.
Long line of diodes
Before: 0.11s
After: 0.10s (probably within margin of error)
Block of wires
I turned the block on and off 10 times.
Before: 1.7s
After: 0.8s
256 byte memory bank
Before: 1.5s
After: 1.3s
Tests
I've done manual testing and things work. The tests from #605 also pass.