Difference between revisions of "Garbage Reduction"

From Kerbal Space Program Wiki
Jump to: navigation, search
(Refined singleton advice.)
(avoid foreach)
Line 16: Line 16:
 
==Best practices to avoid creating garbage==
 
==Best practices to avoid creating garbage==
  
===avoid foreach===
+
=== foreach ===
Replace "foreach" with "for".
+
Use of the foreach loop introduces overhead, especially in previous versions of unity (prior to 5.5, KSP 1.4 updated to Unity 2017.1). Traditional for loops can save the allocation of an enumerator object in certain cases and execute slightly faster at times because they can omit a method call (which may or may not be inlined depending on the situation).
  
Less garbage, but goes through the list backwards:
+
In modern Unity development, this advice is highly situational, however. To illustrate, some code from the [https://github.com/UbioWeldingLtd/UbioWeldContinued UbioWeldContinued mod]:
 
<syntaxhighlight>
 
<syntaxhighlight>
for (int i = <something>.Length - 1; i >= 0; --i)
+
string[] breakingAttributes = new string[newModule.values.DistinctNames().Count()];
 +
for (int i = 0; i < newModule.values.DistinctNames().Count(); i++)
 +
{
 +
    breakingAttributes[i] = string.Concat(newModuleName, Constants.underline, newModule.values.DistinctNames()[i]);
 +
}
 
</syntaxhighlight>
 
</syntaxhighlight>
 
+
Here, the use of linq and piecemeal string manipulations are not the biggest performance issue. The biggest issue is the repeated expression `newModule.values.DistinctNames()[i]`. Not only is it possible for an attribute like `values` to create a new container (list, array, etc.) to return each time it is called, calling `DistinctNames` over and over in the for loop and then indexing into it to retrieve one item is going to create and immediately discard collections of objects for no gain. In this case, a coding style that prefers `foreach` and evaluates performance critical sections of code for possible improvements would have been significantly better.  This is a great example of how indexing into some collection over and over can introduce unintended allocation and performance consequences.
One extra variable, but goes through the list forwards:
 
<syntaxhighlight>
 
int count = <something>.Length; // (or .Count or whatever works for the object concerned)
 
for (int i = 0; i < count; ++i)
 
</syntaxhighlight>
 
 
 
If you're using Visual Studio instead of MonoDevelop, replacing foreach is not strictly necessary in some cases:
 
 
 
"foreach" can be ok in Visual Studio [http://forum.kerbalspaceprogram.com/index.php?/topic/7542-the-official-unoffical-quothelp-a-fellow-plugin-developerquot-thread/&do=findComment&comment=2677079 "if the GetEnumerator function of the object you are doing it on returns a struct type.  If it returns a class type or an interface then it will cause garbage with any compiler.  System classes like List<T> do return a struct but some of the KSP classes that are basically a wrapper around a List<T> return an interface (e.g. PartResourceList).  These are on the list for fixing for KSP 1.2 though with that sort of nesting, you will probably get better performance with plain for loops (using foreach also requires the code to make two function calls to the enumerator object every time around the loop)."]
 
  
 
[http://forum.kerbalspaceprogram.com/index.php?/topic/148448-12pre-gravityturn-continued-automated-efficient-launches/&do=findComment&comment=2779177 "You don't need to convert all your foreach loops into plain for loops unless the thing being enumerated happens to return an interface or class from GetEnumerator (even Squad no longer *need* to avoid foreach because Unity have updated the included compiler to fix the issue)."]
 
[http://forum.kerbalspaceprogram.com/index.php?/topic/148448-12pre-gravityturn-continued-automated-efficient-launches/&do=findComment&comment=2779177 "You don't need to convert all your foreach loops into plain for loops unless the thing being enumerated happens to return an interface or class from GetEnumerator (even Squad no longer *need* to avoid foreach because Unity have updated the included compiler to fix the issue)."]
  
However if you want a hard and fast rule, "don't use foreach" is pretty simple.
 
 
=== Singletons ===
 
=== Singletons ===
  

Revision as of 20:15, 14 June 2018

Some of this information may be incorrect, speculative or out of date. Unity provides an in-depth tutorial with more up to date and accurate information. As of version 1.4, Kerbal Space Program runs on Unity 2017.1, which, for example, reduces the concern about foreach loops (problematic for Unity versions before 5.5). All advice is situational and the largest source of garbage collection issues in most user made plugins will be lack of/poor design.

What is garbage, and why is it bad?

C# is a garbage-collected language, which means that the programmer doesn't have to worry about deleting variables when done with them; the system will do it automatically.

However, the way it does this is by freezing the system and then walking through the list of variables and removing the unused ones ("garbage" variables). This can cause stutters, and the stutters will be more frequent and longer the more garbage there is.

Measuring garbage production

MemGraph can show you how much garbage KSP is creating. It is not broken down by mod, so to test a specific mod you'll need a copy of KSP with only that mod installed.

What does Squad say about garbage?

"removing old foreach() loops, Singletons and generally de-linqing the game. We recommend that modders to the same in anticipation of the next update to help increase performance for all KSP players. Linq and foreach() loops in particular create a lot of garbage, which is what causes the stutter that some people experience while running the game."

Best practices to avoid creating garbage

foreach

Use of the foreach loop introduces overhead, especially in previous versions of unity (prior to 5.5, KSP 1.4 updated to Unity 2017.1). Traditional for loops can save the allocation of an enumerator object in certain cases and execute slightly faster at times because they can omit a method call (which may or may not be inlined depending on the situation).

In modern Unity development, this advice is highly situational, however. To illustrate, some code from the UbioWeldContinued mod:

string[] breakingAttributes = new string[newModule.values.DistinctNames().Count()];
for (int i = 0; i < newModule.values.DistinctNames().Count(); i++)
{
    breakingAttributes[i] = string.Concat(newModuleName, Constants.underline, newModule.values.DistinctNames()[i]);
}

Here, the use of linq and piecemeal string manipulations are not the biggest performance issue. The biggest issue is the repeated expression `newModule.values.DistinctNames()[i]`. Not only is it possible for an attribute like `values` to create a new container (list, array, etc.) to return each time it is called, calling `DistinctNames` over and over in the for loop and then indexing into it to retrieve one item is going to create and immediately discard collections of objects for no gain. In this case, a coding style that prefers `foreach` and evaluates performance critical sections of code for possible improvements would have been significantly better. This is a great example of how indexing into some collection over and over can introduce unintended allocation and performance consequences.

"You don't need to convert all your foreach loops into plain for loops unless the thing being enumerated happens to return an interface or class from GetEnumerator (even Squad no longer *need* to avoid foreach because Unity have updated the included compiler to fix the issue)."

Singletons

Singletons are often designed to survive the life of the program's execution. This long life means they may hold references to objects long after they are useful which can result in the objects never being released (a memory leak), surviving long enough that they have to be considered by the garbage collector in a more expensive generation (slowing GC over time), or deallocated after a similar set of objects has been allocated in another spot on the heap (memory fragmentation). It is therefore important to carefully evaluate the objects a singleton can reference. This can include more than the immediate attributes of the singleton, i.e. those attributes may hold references to other objects.

remove "using System.Linq"

Linq creates a lot of garbage. Don't use it.

"the quickest way to check if you are using Linq is to delete that using line if it's there. Then everything with Linq will have errors. And there are many more things other than the From, Select that use Linq; lots of methods that are used to modify or specify certain members of a list use it, like Any, OfType, Where, First, FirstOrDefault, etc..."

don't loop through all Parts/Resources/whatever every frame

Looping through every part on the ship is expensive. Don't do this every frame, but instead get notified when the ship changes and then re-scan it. Same for resources, or anything else that you can cache and then get notified when you need to act.

List<Part> partsICareAbout = new List<Part>();
 
Start()
{
  rescanMethod(); // run this at startup
 
  GameEvents.onVesselChange.Add(rescanMethod);
  // onVesselChange - switching between vessels with [ or ] keys
 
  GameEvents.onVesselStandardModification.Add(rescanMethod);
  // onVesselStandardModification collects various vessel events and fires them off with a single one.
  // Specifically - onPartAttach,onPartRemove,onPartCouple,onPartDie,onPartUndock,onVesselWasModified,onVesselPartCountChanged
}
 
public void rescanMethod(Vessel gameEventVessel=null)
{
  // scan the ship for the parts you need, and save them in partsICareAbout
}