kikito / bump.lua

A collision detection library for Lua
MIT License
922 stars 86 forks source link

Various Garbage Generation Optimizations #53

Open oniietzschan opened 4 years ago

oniietzschan commented 4 years ago

Hey kikito,

Here's a great big PR with various changes I've made in service of reducing the amount of garbage generated by Bump. Some of these changes I would expect to slightly increase the CPU time used by certain operations. However, in my experience garbage collection tends to be big bottleneck for keeping frametimes low, so in my view this is still a very worthwhile tradeoff. Hopefully you'll agree.

I'm going to be going through and leaving comments explaining the changes I've made. So if you happen to catch this right as I send it, you'll probably want to wait for me to finish writing those comments.

I've included the benchmark which I used while making these changes. It inserts a bunch of objects and world:move()s them around for a couple hundred generations. It's random, but is set to used a fixed seed, so the operations it performs should be identical across tests. By my testing, my changes cause bump generate less than 1/3rd of the garbage it used to. Here are the results I get on my machine:

anzu@Toki:~/git/bump$ luajit -v
LuaJIT 2.1.0-beta3 -- Copyright (C) 2005-2017 Mike Pall. http://luajit.org/

anzu@Toki:~/git/bump$ luajit performance_test.lua
============= Original =============
Garbage: 1741.58 kB
(Average after 10 tests.)
============= Modded =============
Garbage: 529.91 kB
(Average after 10 tests.)
anzu@Toki:~/git/bump$

anzu@Toki:~/git/bump$ lua -v
Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio

anzu@Toki:~/git/bump$ lua performance_test.lua
============= Original =============
Garbage: 3086.53 kB
(Average after 10 tests.)
============= Modded =============
Garbage: 996.72 kB
(Average after 10 tests.)

For what it's worth, these changes are live in my game Vacant Kingdom on Steam, which has been played (to some extent...) by several thousand people at this point. No bugs related to this have come to my attention, so I'd consider these changes to be fairly safe. The unit tests are passing too, of course.

Please have a look through and let me know which changes you're okay with, and I'll send you an updated PR with just those changes, okay?

Thanks, shru

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.9%) to 97.344% when pulling 100641a01fb71f05ac611c2e5e73b1e98d7c041f on oniietzschan:2d into 7cae5d1ef796068a185d8e2d0c632a030ac8c148 on kikito:master.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.8%) to 97.229% when pulling d01d827e26ea992b28f2cadee5e3a85c71c48307 on oniietzschan:2d into 7cae5d1ef796068a185d8e2d0c632a030ac8c148 on kikito:master.