Refactor `CalculateBallisticAcceleration`

Garanas commented 2 years ago

Description

The base game has an issue with the engine where a bomber can consistently miss its target. This is fixed by manually computing the trajectory to the target. This is done by this file. The file is written with the ease of the developer in mind. With this issue we refactor it for better performance. The input is a weapon and a projectile fired by that weapon. The output is a float value that represents the adjusted y-acceleration. Anything in between needs to be as optimal as possible.

I've attempted optimizing this file in the past but made a mistake somewhere: https://github.com/FAForever/fa/pull/3509 . Feel free to use that branch as your base or to start your own branch from scratch. I got lost because I also tried to fix https://github.com/FAForever/fa/issues/2407 - shouldn't have done those at the same time 😄 .

Course of action

The function in question allocates quite a few tables, performs various inefficient engine calls and is riddled with the GETTABLE lua byte code operation. This list is by no means exhaustive, but to sum up a few goals:

Prevent use of tables when possible, use locals instead. As an example https://github.com/FAForever/fa/pull/3738 where a lot of the table work is replaced with locals
Upvalue engine functions and globals accordingly, preferably there are no GETGLOBAL calls in the byte code anymore

Test plan

The script is reloaded each time a projectile is created. This makes it easier to test. Best to capture the entire function in a pcall to prevent a potential crash from messing up the game state.

By all means the result of the function must be the same. E.g., given a weapon w and a projectile p then we should have CalculateBallisticAcceleration(w, p) == CalculateBallisticAcceleration'(w, p) where CalculateBallisticAcceleration' is the optimized version of the function.

Learning goals

Feel more comfortable working on complicated code.

Other

By adding LOG(repr(debug.listcode(CalculateBallisticAcceleration))) to the bottom of the file you can list the byte code in the logs. Can't wait to compare it with the optimized version 😄 .

{ "( 8) 0 - SELF 2 0 250", "( 8) 1 - CALL 2 2 2", "( 9) 2 - GETTABLE 3 2 251", "( 10) 3 - GETTABLE 4 2 252", "( 11) 4 - LOADK 5 3", "( 12) 5 - SELF 6 1 254", "( 12) 6 - CALL 6 2 2", "( 13) 7 - TEST 6 6 1", "( 13) 8 - JMP 0 1", "( 13) 9 - RETURN 5 2 0", "( 14) 10 - GETTABLE 7 6 255", "( 18) 11 - NEWTABLE 8 0 2", "( 18) 12 - SELF 9 1 257", "( 18) 13 - CALL 9 2 2", "( 18) 14 - SETTABLE 8 256 9", "( 18) 15 - GETGLOBAL 9 9", "( 18) 16 - GETGLOBAL 10 10", "( 18) 17 - SELF 11 6 261", "( 18) 18 - CALL 11 2 0", "( 18) 19 - CALL 10 0 2", "( 18) 20 - LOADK 11 12", "( 18) 21 - CALL 9 3 2", "( 18) 22 - SETTABLE 8 258 9", "( 19) 23 - SELF 9 6 263", "( 19) 24 - CALL 9 2 2", "( 21) 25 - LOADNIL 10 10 0", "( 22) 26 - TEST 9 9 0", "( 22) 27 - JMP 0 19", "( 22) 28 - GETGLOBAL 11 14", "( 22) 29 - MOVE 12 9 0", "( 22) 30 - CALL 11 2 2", "( 22) 31 - TEST 11 11 0", "( 22) 32 - JMP 0 14", "( 24) 33 - NEWTABLE 11 0 2", "( 24) 34 - SELF 12 9 257", "( 24) 35 - CALL 12 2 2", "( 24) 36 - SETTABLE 11 256 12", "( 24) 37 - GETGLOBAL 12 9", "( 24) 38 - GETGLOBAL 13 10", "( 24) 39 - SELF 14 9 261", "( 24) 40 - CALL 14 2 0", "( 24) 41 - CALL 13 0 2", "( 24) 42 - LOADK 14 12", "( 24) 43 - CALL 12 3 2", "( 24) 44 - SETTABLE 11 258 12", "( 24) 45 - MOVE 10 11 0", "( 24) 46 - JMP 0 11", "( 27) 47 - NEWTABLE 11 0 2", "( 27) 48 - SELF 12 0 265", "( 27) 49 - CALL 12 2 2", "( 27) 50 - SETTABLE 11 256 12", "( 27) 51 - GETGLOBAL 12 10", "( 27) 52 - LOADK 13 16", "( 27) 53 - LOADK 14 16", "( 27) 54 - LOADK 15 16", "( 27) 55 - CALL 12 4 2", "( 27) 56 - SETTABLE 11 258 12", "( 27) 57 - MOVE 10 11 0", "( 30) 58 - LT 0 267 3", "( 30) 59 - JMP 0 11", "( 30) 60 - GETGLOBAL 11 18", "( 30) 61 - GETTABLE 11 11 7", "( 30) 62 - EQ 0 11 269", "( 30) 63 - JMP 0 7", "( 31) 64 - GETGLOBAL 11 18", "( 31) 65 - NEWTABLE 12 0 2", "( 31) 66 - SETTABLE 12 270 253", "( 31) 67 - SETTABLE 12 271 3", "( 31) 68 - GETTABLE 13 10 256", "( 31) 69 - SETTABLE 12 272 13", "( 31) 70 - SETTABLE 11 7 12", "( 34) 71 - GETGLOBAL 11 18", "( 34) 72 - GETTABLE 11 11 7", "( 36) 73 - GETTABLE 12 10 256", "( 36) 74 - TEST 12 12 0", "( 36) 75 - JMP 0 3", "( 36) 76 - GETTABLE 12 11 273", "( 36) 77 - TEST 12 12 0", "( 36) 78 - JMP 0 14", "( 37) 79 - TEST 11 11 0", "( 37) 80 - JMP 0 11", "( 39) 81 - GETTABLE 5 11 270", "( 40) 82 - GETTABLE 12 11 271", "( 40) 83 - SUB 12 12 267", "( 40) 84 - SETTABLE 11 271 12", "( 41) 85 - LOADBOOL 12 1 0", "( 41) 86 - SETTABLE 11 273 12", "( 43) 87 - GETTABLE 12 11 271", "( 43) 88 - LT 0 12 267", "( 43) 89 - JMP 0 2", "( 44) 90 - GETGLOBAL 12 18", "( 44) 91 - SETTABLE 12 7 269", "( 48) 92 - RETURN 5 2 0", "( 52) 93 - NEWTABLE 12 0 2", "( 52) 94 - GETUPVAL 13 0 0", "( 52) 95 - GETTABLE 14 8 256", "( 52) 96 - GETTABLE 15 10 256", "( 52) 97 - CALL 13 3 2", "( 52) 98 - SETTABLE 12 256 13", "( 52) 99 - GETUPVAL 13 0 0", "( 52) 100 - GETTABLE 14 8 258", "( 52) 101 - GETTABLE 15 10 258", "( 52) 102 - CALL 13 3 2", "( 52) 103 - SETTABLE 12 258 13", "( 53) 104 - GETTABLE 13 12 258", "( 53) 105 - EQ 0 13 266", "( 53) 106 - JMP 0 1", "( 53) 107 - RETURN 5 2 0", "( 55) 108 - GETTABLE 13 2 274", "( 55) 109 - TEST 13 13 0", "( 55) 110 - JMP 0 10", "( 57) 111 - GETTABLE 13 12 256", "( 57) 112 - GETGLOBAL 14 25", "( 57) 113 - GETTABLE 14 14 276", "( 57) 114 - GETTABLE 15 2 274", "( 57) 115 - SUB 15 267 15", "( 57) 116 - LOADK 16 16", "( 57) 117 - LOADK 17 17", "( 57) 118 - CALL 14 4 2", "( 57) 119 - MUL 13 13 14", "( 57) 120 - SETTABLE 12 256 13", "( 60) 121 - GETGLOBAL 13 18", "( 60) 122 - GETTABLE 13 13 7", "( 60) 123 - EQ 1 13 269", "( 60) 124 - JMP 0 29", "( 63) 125 - GETTABLE 13 12 258", "( 63) 126 - MUL 13 4 13", "( 63) 127 - MUL 13 13 277", "( 64) 128 - GETGLOBAL 14 18", "( 64) 129 - GETTABLE 14 14 7", "( 64) 130 - GETTABLE 14 14 271", "( 64) 131 - SUB 14 3 14", "( 67) 132 - GETTABLE 15 12 256", "( 67) 133 - SUB 16 3 267", "( 67) 134 - MUL 16 13 16", "( 67) 135 - DIV 16 16 278", "( 67) 136 - SUB 15 15 16", "( 67) 137 - MUL 16 13 14", "( 67) 138 - ADD 15 15 16", "( 67) 139 - SETTABLE 12 256 15", "( 68) 140 - GETGLOBAL 15 18", "( 68) 141 - GETTABLE 15 15 7", "( 68) 142 - GETGLOBAL 16 18", "( 68) 143 - GETTABLE 16 16 7", "( 68) 144 - GETTABLE 16 16 271", "( 68) 145 - SUB 16 16 267", "( 68) 146 - SETTABLE 15 271 16", "( 69) 147 - GETGLOBAL 15 18", "( 69) 148 - GETTABLE 15 15 7", "( 69) 149 - GETTABLE 15 15 271", "( 69) 150 - LT 0 15 267", "( 69) 151 - JMP 0 2", "( 70) 152 - GETGLOBAL 15 18", "( 70) 153 - SETTABLE 15 7 269", "( 76) 154 - GETTABLE 13 12 256", "( 76) 155 - GETTABLE 14 12 258", "( 76) 156 - DIV 13 13 14", "( 77) 157 - EQ 0 13 266", "( 77) 158 - JMP 0 1", "( 77) 159 - RETURN 5 2 0", "( 80) 160 - NEWTABLE 14 3 0", "( 80) 161 - GETTABLE 15 10 256", "( 80) 162 - GETTABLE 15 15 267", "( 80) 163 - GETTABLE 16 10 258", "( 80) 164 - GETTABLE 16 16 267", "( 80) 165 - MUL 16 13 16", "( 80) 166 - ADD 15 15 16", "( 80) 167 - LOADK 16 16", "( 80) 168 - GETTABLE 17 10 256", "( 80) 169 - GETTABLE 17 17 280", "( 80) 170 - GETTABLE 18 10 258", "( 80) 171 - GETTABLE 18 18 280", "( 80) 172 - MUL 18 13 18", "( 80) 173 - ADD 17 17 18", "( 80) 174 - SETLIST 14 2", "( 80) 175 - SETTABLE 10 279 14", "( 82) 176 - GETTABLE 14 10 279", "( 82) 177 - GETGLOBAL 15 31", "( 82) 178 - GETTABLE 16 10 279", "( 82) 179 - GETTABLE 16 16 267", "( 82) 180 - GETTABLE 17 10 279", "( 82) 181 - GETTABLE 17 17 280", "( 82) 182 - CALL 15 3 2", "( 82) 183 - SETTABLE 14 278 15", "( 90) 184 - GETGLOBAL 14 25", "( 90) 185 - GETTABLE 14 14 282", "( 90) 186 - DIV 15 267 13", "( 90) 187 - LOADK 16 28", "( 90) 188 - CALL 14 3 2", "( 90) 189 - MUL 14 278 14", "( 90) 190 - GETTABLE 15 8 256", "( 90) 191 - GETTABLE 15 15 278", "( 90) 192 - GETTABLE 16 10 279", "( 90) 193 - GETTABLE 16 16 278", "( 90) 194 - SUB 15 15 16", "( 90) 195 - MUL 5 14 15", "( 92) 196 - GETGLOBAL 14 18", "( 92) 197 - GETTABLE 14 14 7", "( 92) 198 - TEST 14 14 0", "( 92) 199 - JMP 0 3", "( 94) 200 - GETGLOBAL 14 18", "( 94) 201 - GETTABLE 14 14 7", "( 94) 202 - SETTABLE 14 270 5", "( 97) 203 - RETURN 5 2 0", "( 98) 204 - RETURN 0 1 0", maxstack=19, numparams=2 }

Hdt80bro commented 2 years ago

@jkstpierre are you handling this? Otherwise, it looks like a lovely thing to work on heheheh... I'll try to keep it readable.

Garanas commented 2 years ago

Hold your toes - I may be refactoring this entire approach with Spikey of the balance team.

Hdt80bro commented 2 years ago

Aww, alright. Enjoy~

FAForever / fa