FAForever / fa

Lua code for FAF
223 stars 230 forks source link

Remove usage of `repr` in production code #6067

Closed Garanas closed 4 months ago

Garanas commented 4 months ago

Description of the proposed changes

Related to https://github.com/FAForever/fa/issues/5946, #4872 and https://github.com/FAForever/fa/pull/6050

The use of repr is extremely dangerous from a computational perspective and should only be used when debugging. Due to the recursive nature it can try to create a string of many, many more tables than just the current table reference.

Also removed the cycle counting of the inspect library. The computation occurred but the output was not used.

Testing done on the proposed changes

Run several AI games.

Additional context

@relent0r reported stutters since #6050 , this is because of the following line:

safecall("Unable to IssueTransportLoad units are "..repr(units), IssueTransportLoad, newunits, transport )

Because Lua evaluates eagerly the units were recursively turned into a string, regardless of whether the call succeeded or not. In FAF this also includes various references to the blueprint(s) and the brain. Therefore this can be an expensive operation! It's a miracle that this previously did not generate (noticeable) stutters!

Checklist

Garanas commented 4 months ago

AI vs AI games are noticeable faster with the relevant code fixed 😃 !