search

ZeroK-RTS / Zero-K

Open source RTS game running on the Spring/Recoil engine
https://zero-k.info
GNU General Public License v2.0
674 stars 204 forks source link

Widget/Gadget handlers use ipairs everywhere #816

Closed sprunk closed 9 years ago

sprunk commented 9 years ago

Apparently ipairs are expensive. Should that be changed to normal iteration or is keeping ipairs intended?

xponen commented 9 years ago

@GoogleFrog did a messy UnitPreDamaged() optimization, judge from that, and decide if you want it on all callins

sprunk commented 9 years ago

That optimisation is unrelated to the removal of ipairs (it exists to prevent calling the event for gadgets that declare they aren't interested) and is not even messy. It could even be as tidy as all other call-ins if the initialisation stuff actually happened on initialisation instead of checking for the first run every time.

Replacing ipairs with normal iteration is still fairly tidy but even if it wasn't, gadget handler can afford to be messy if it helps gain performance.

GoogleFrog commented 9 years ago

I think you should run a test to determine the difference between ipairs and a manual iterator. I think tidyness is important for the gadget handler but I would also like to keep the difference between the basecontent handler and ours small. If there is no performance gain to be had with this change then I do not want it implemented.

aeonios commented 9 years ago

About that, using normal iteration is faster, but only if the iterator is the most expensive part of a given loop. In other words, it only matters if the loop does 9001 iterations but has a cheap body. I experimented with it in my own code and never noticed a difference.

xponen commented 9 years ago

there won't be any way to prove this will work because there's no gadgetHandler or widgetHandler profiler.

sprunk commented 9 years ago

According to https://springrts.com/wiki/Lua_Performance normal iteration is twice as fast. The overhead is comparatively small in most cases however.

There are some call-ins that tend to have cheap bodies and get called a lot and so could benefit from the reduced overhead (for example Allow*BuildStep).

sprunk commented 9 years ago

Closing because any changes in the handlers are difficult to measure.