KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
699 stars 230 forks source link

Vecdraw memory leak #2376

Open luovahulluus opened 5 years ago

luovahulluus commented 5 years ago
clearscreen.
CLEARVECDRAWS().
SET CONFIG:IPU TO 2000.

Until false
{
    set vf to vecdraw(v(0,0,0), srfprograde:vector*5, yellow, "", 1.0, true, 0.2).
    // wait 0.
}

This code causes the game to use more and more memory until it runs out and the game crashes. With the wait 0. line uncommented the effect is a lot smaller, but still about 1MB per 2 seconds. Setting the true to false, removes the problem (and the arrow, of course). KSP.log I'm running KSP 1.3.1.1891 with kOS 1.1.5.0.

Dunbaratu commented 5 years ago

If you try it adding this one line, does it behave any differently?

Until false
{
        set vf to 0. // <<< extra inserted line.
    set vf to vecdraw(v(0,0,0), srfprograde:vector*5, yellow, "", 1.0, true, 0.2).
    // wait 0.
}

Doing that would cause it to explicitly orphan the previous vecdraw object just before creating a new one, rather than having to wait for C#'s runtime to eventually notice it got orphaned several seconds later the next time it runs its mark/sweep orphan checker.

The reason for me asking you to try this test is that I suspect the problem might merely be that the vecdraw objects are in fact being freed, but at a slower pace than the pace at which they are getting created. If the loop wasn't infinite and instead eventually stopped, that stop would give the garbage collector time to catch up and free everything.

Actually that would be a good second test. Take out the set vf to 0. again, then have it be a loop that merely runs a finite but large number of iterations instead of an infinite number of them (large enough to grow a big memory footprint, but not a big enough one to run out of memory and crash the program). Then see if the memory usage falls back down again eventually several seconds after it's over.

Keep in mind if doing this, the big problems with trying to get a true picture of the memory footprint of a C# program. It's not easy because the VM will often not release "freed" heap memory until it actually has to because some other program is asking for it. Even without kOS installed at all, the stock KSP game appears to grow its memory footprint the longer you run it.

There is a possibility of a problem in kOS. I'm just trying to get the bigger picture of what is happening first. One possible problem is that when you wrap a Unity object inside a C# class, you can cause memory leak because that Unity object has quite a few C++ (not C#) objects inside it that don't obey the "orphaned = freed" rule of managed memory systems that C# programmers will just assume are in play. Sometimes the C# class needs a finalizer that manually disposes of the Unity objects inside it. There's a chance we're not catching that with the Unity LineRenderer's that vecdraw uses.

luovahulluus commented 5 years ago

(With the wait 0. line uncommented) the memory increase rate seems to be pretty much the same with "set vf to 0." and without it.

clearscreen.
CLEARVECDRAWS().
SET CONFIG:IPU TO 2000.
set x to 0.

Until x = 1000000
{
    //set vf to 0.
    set vf to vecdraw(v(0,0,0), srfprograde:vector*5, yellow, "", 1.0, true, 0.2).
    //wait 0.
    set x to x + 1.
    print x at(5,5).
}
wait 120.

This increased my memory usage from 1.7GB to 10GB during the main loop. After x reached 1 000 000 memory usage didn't change, it held steady at 10GB atleast 5 minutes.

Dunbaratu commented 5 years ago

Okay well that might be a problem then. With the .Net runtime you can just never tell how much of the memory footprint you see in something like Task Manager in Windows or "top" in UNIX is psuedo-freed heap that is ready to be re-used if the program needs it, versus how much is actually required memory the program cannot relinquish. All you can really tell is that once upon a time it used 10GB. You can never till if it still is really needing all that or if it just is choosing not to bother releasing it until the computer is starting to run out of memory and it has to.

But the fact that it does seem to crash if you let it keep going seems to suggest it might be a "real" leak and it really is using that 10GB.

I'll have to start looking at vecdraw and seeing if it's failing to dispose of some Unity objects. The set vf to 0. line does force it to remove the vecdraw, so if that didn't get rid of the leak that probably means vecdraw leaves some Unity objects still around when it gets deleted. (It creates two LineRenderers (the shaft and the hat of the arrow), and one TextLabel. Any of those might be the culrpit.)

luovahulluus commented 5 years ago

If I haven't been using vecdraws, loading a savegame is fairly quick. If I have used them to the point the game uses 10GB of memory, loading can take a minute or three (estimated). To me that seems like the problem is real (but I don't know much of anything of these things). After loading, the memory usage is normal again.

Dunbaratu commented 5 years ago

Weird. But understandable. Here's an interesting question - after you do those long load times, what does the memory footprint look like? Scene switching in Unity makes Unity dispose pretty much all its objects and start building them again from scratch. So that might end up throwing away all the vecdraw leakage.

luovahulluus commented 5 years ago

Loading the game returns the memory usage back to normal.

hvacengi commented 5 years ago

I have long suspected that there is an issue with vecdraws and memory, but I've done testing to confirm that the instances are garbage collected and at the time I tested it they were. I wonder if it's a matter of an instance variable in the vecdraw that isn't getting GC'ed for some reason. If it does turn out that the initial syntax isn't triggering our orphan monitoring, that's an issue that should be addressed, but I'm pretty sure I tested that too. No matter how you slice it though, kOS shouldn't be allocating enough objects to let 10GB of memory get allocated, even if the runtime doesn't shrink the memory after is is released (I believe neither .net nor Mono have a mechanism to reduce that allocation automatically, it just grows any time the runtime needs more memory).

I can do some further testing this weekend if it is needed.