KSP 1.4.1 Performance

[Lisias]

2 hours ago, DMagic said:

That's the whole problem with IMGUI. You recreate the entire UI multiple times every frame.

If I understood the documentation correctly, they are intended for being use outside from a event driven UI main loop.

 

2 hours ago, DMagic said:

I'm not sure what this means as it relates to IMGUI. 

In PHP, you recreate the World each Request (including SGDB connections, unless you use a Connection Manager, that by definition is external to the PHP.exe).

Until the moment, I have evidences that on Unity, the IMGUI appears to work on the following life-cycle: you have a Awake(), when your MonoBehaviour is created, then a Start(). A Reset() happens everytime your Behaviours gets the focus, and then later a OnGUI(), called everytime it's your time to draw something in the screen (this sounds a lot as a Stacking Window Manager).

But on the very Unity Documentation for OnGUI, you find a example where the GUILayout is used. =/ I know well a lot of things, but Unity is not (yet) one of them - but or someone on Unity's documentation team screwed up epically, or the OnGUI is being handled somewhat heterodoxly by us. However, here we have the Unity's life-cycle, where it's clear that OnGUI is called multiple times (so, it's not my "OnFocus", Reset() is the correspondent one). Ergo, yeah - the example I linked above is... not exactly the best one possible. IMGUI should not recreate everything each time OnGUI is called, it's plain nuts.

2 hours ago, DMagic said:

don't ask me about all of that SCAN_MBW/MBE stuff, I didn't write that, though I know it has its origin in TriggerAu's plugin framework code; it has all since been removed),

For a good reason: this thing essentially implements a Stacking Event Driven UI in each GameObject. :-) Since you decided to implement your UI in separated GameObjects, all that clutter is just making your life miserable giving you nothing in exchange.

2 hours ago, DMagic said:

I don't see how you can cache or store any of that information. Recreating it on every frame is just how it works, unless there is some alternate way of doing it that I am completely unaware of. You can, of course, cache certain components of the UI, strings (or StringBuilder instances), textures, etc... but if you want to draw a button you just have to call GUI/GUILayout.Button on every frame.

What's precisely the worst possible way of doing things, from the performance point of view. If I need a button, I need a container for it. So, I create a Container, create the button inside it, configure the button attributes (size, position on the container, event handlers as "OnClick"), put everything hooked on a variable on the "Start()" and then, on the OnGUI, just tell Unity to "draw this container on this position". Such container is destroyed on the "OnDestroy" event, or you will have a memory leak.

Somewhat more complicated life cycles can be implemented for Widgets that are not necessary all the time. Sometimes you need a Settings, other a Part Atribute Editor, etc, so in order to prevent having everything and the kitchen's sink created all the time (cluttering the heap and increasing the minimum memory footprint for your code), we build a FSM on the OnGUI to handle the mess. 

Of course, I'm telling you what we did on the Win16, Win32 and even Win64 (QT does the same, Android too - but in a highly abstracted way). Unity appears to be somewhat exoteric, as it appears.

2 hours ago, DMagic said:

I don't see where UbioWeld is doing it any differently. It looks like it has a lot of data stored in constants, but its UI code is still being drawn with GUILayout, and not every bit of text, or component of the UI can be cached at compile time like that.

Here, on UbioZurWeldingLtd.cs, you can see the original author controlling states and creating "high level widgets" on the object to be reused. Follow all the references for the "_welder" attribute and you will see the guy "caching" some widgets. However, and you have a strong point that I didn't paid the due attention, there's a lot of calls to the methods you are complaining. 

Speaking frankly, this appears to be a memory bias from my side. =/

Edited April 24, 2018 by Lisias
bad grammar

[Lisias]

1 hour ago, DMagic said:

For most of my UI's I destroy everything when the window is closed. Creating and destroying the UI isn't generally that intensive, and it simplifies some things. For some, like Contracts Window +, the opening process is particularly intensive (meaning poorly thought out and coded ) so I just hide it when the window is closed.

The thing is not exactly about spending CPU juice, but cluttering the heap.

This is a problem even on the old and faithful ANSI C, when we used to write GUI on this thing (and yes, I also wrote programs in TurboVision using TurboC :-D ). By (ab)using from the malloc and free calls, sooner or later (and on a "640k should be enough to everyone" era, the later was sooner than one would expect), the LIBC would fail due excessive heap fragmentation (without virtual memory, and without "indirect pointers", there was no way to defragment the heap!).

What IMGUI is doing is, essentially, (ab)using the heap. :-(

And on a Mono application (as well Java, another thing that I know very well), abusing the heap is calling the GC for a fight.

1 hour ago, DMagic said:

There are a few different ways to go about hiding a UI. You can just turn off the GameObject using SetActive, which stops all of the attached scripts from running and hides the window, but calling SetActive itself can be a problem.

Another way is to use a CanvasGroup script attached to the window. If you set the alpha for a CanvasGroup down to 0, and make it non-interactable and not block ray casts, then (supposedly, I don't think I've every actually tried this) all child objects in the CanvasGroup will be ignored by the UI (they won't generate any draw calls), but the attached scripts will still be running, so it could be a trickier way of handling hiding the UI.

I will take a close look on all of this.

The "transparent" stunt, however, appears to have his days counted. Everybody is (or plans) to use ClickThroughBlocker, I'm right? (I just *hate* when a a click pass through the button and acts on something behind it!)

[DMagic]

24 minutes ago, Lisias said:

If I need a button, I need a container for it. So, I create a Container, create the button inside it, configure the button attributes (size, position on the container, event handlers as "OnClick"), put everything hooked on a variable on the "Start()" and then, on the OnGUI, just tell Unity to "draw this container on this position".

That's essentially what the new UI does. You define all of the attributes of the UI element when you create it, or before, then the canvas system just passes it to the GPU every frame, rather than recreating everything. I see some things that look like cached UI objects in UbioWeld, but I think those are just workarounds for a the lack of any built-in drop down menu (another thing that the new UI does nothing to solve, along with tooltips; thankfully there are lots of helpful extensions available for things like this).

Everything is event-driven, so instead of testing whether a button is being pressed multiple times per frame by calling GUILayout.Button, you just assign a listener to the Button onClick handler, then when you click the button it detects the mouse click and invokes the event and all of its listeners. You can still update things on a regular basis, using the Update method, or a CoRoutine, or something similar, but you don't have to.

 

30 minutes ago, Lisias said:

IMGUI should not recreate everything each time OnGUI is called, it's plain nuts.

Indeed it is. Even before Unity 4.6, the IMGUI system was never really sufficient for any kind of complex game UI, and basically unusable for mobile. Before that everyone was basically completely reliant on third-party UI system developers (one of which, I think, was used as the basis for the current system) to produce anything workable. KSP used 1 or 2 of these systems in the past, along with IMGUI, replacing all of that was a major part of the upgrade from KSP 1.0 to 1.1.

 

18 minutes ago, Lisias said:

The "transparent" stunt, however, appears to have his days counted. Everybody is (or plans) to use ClickThroughBlocker, I'm right? (I just *hate* when a a click pass through the button and acts on something behind it!)

Click through is another are where IMGUI falls apart completely. All of those UI elements live in a different world from the rest of the Unity objects, so a window, or button, or any other UI element does nothing to stop the mouse click events from passing through to other objects. The fact that all IMGUI elements are drawn on top of everything else doesn't help now that all of the stock UI has moved to the new system.

ClickThroughBlocker, or similar systems used by individual mods, is a workaround that helps. But the new UI system avoids the problem completely, the UI elements behave the same as other objects in the game and block mouse click events (if the element doesn't need interaction you can make them transparent to raycasts, if you want to). The exceptions here would be for text input fields, where you need to manually lock out KSP controls so that you aren't turning around, or activating the lights or something while typing. And something like keyboard shortcuts for you window, where you might only want the shortcuts to work when the mouse is over the window, or something along those lines. 

The thing about using CanvasGroup isn't really related. If you set the alpha for the canvas group to 0 if makes all UI elements invisible, so you generally wouldn't want them be blocking mouse clicks anymore; you could if you wanted to, but I can't think of many cases where you would.

 

 

[Lisias]

10 minutes ago, DMagic said:

That's essentially what the new UI does. You define all of the attributes of the UI element when you create it, or before, then the canvas system just passes it to the GPU every frame, rather than recreating everything. [...]

Everything is event-driven, so instead of testing whether a button is being pressed multiple times per frame by calling GUILayout.Button, you just assign a listener to the Button onClick handler, then when you click the button it detects the mouse click and invokes the event and all of its listeners. You can still update things on a regular basis, using the Update method, or a CoRoutine, or something similar, but you don't have to.

What leads me to the conclusion that you were right from the beginning. So I stand corrected, my apologies, and a thank you very much for your patience while explaining it to me. :-)

10 minutes ago, DMagic said:

Indeed it is. Even before Unity 4.6, the IMGUI system was never really sufficient for any kind of complex game UI, and basically unusable for mobile. Before that everyone was basically completely reliant on third-party UI system developers (one of which, I think, was used as the basis for the current system) to produce anything workable. KSP used 1 or 2 of these systems in the past, along with IMGUI, replacing all of that was a major part of the upgrade from KSP 1.0 to 1.1.

Well, there's nothing more I can say but "OnGUI system basically useless if you don't already know it. It's almost completely esoteric and has no value outside of legacy Unity UI." © 2018 @DMagic, The Patient. :-)

[xEvilReeperx]

1 minute ago, DMagic said:

The thing about using CanvasGroup isn't really related. If you set the alpha for the canvas group to 0 if makes all UI elements invisible, so you generally wouldn't want them be blocking mouse clicks anymore; you could if you wanted to, but I can't think of many cases where you would. 

Don't do this, it has every drawback and no advantage. They will all be rendered anyways and any change to any element will still dirty its canvas and trigger a rebuild

[DMagic]

10 minutes ago, xEvilReeperx said:

Don't do this, it has every drawback and no advantage. They will all be rendered anyways and any change to any element will still dirty its canvas and trigger a rebuild

Wow, that's a great video (a lot of these Unite videos are really good), now I'm going to be watching that dapper guy for the next hour. I didn't know that bit about all UI being essentially transparent for rendering. I guess overdraw probably isn't much of a problem for KSP though.

But is he referring to CanvasGroup alpha for canvas rebuilds, or just Image alpha? I didn't hear any explicit mention of CanvasGroup. Either way, it still sounds like not the best way to disable a UI.

I think this is where I heard that bit about CanvasGroup alpha (and this actually points to the same video). Like I said, I've never actually tried to use CanvasGroup as a means of disabling a UI, only for fade in and fade out transitions, but their mention of the CanvasGroup alpha set to 0 preventing draw calls seems to bear out. The Frame Debugger shows all of the child meshes dropping out of the draw call stack when the CanvasGroup alpha is set to 0. The same is not true about setting an Image's color field to alpha = 0.

The other thing that link discusses, using Rect Mask 2D instead of a regular Mask for scroll views is a good point. I use it a lot and it can make a huge difference. I still don't quite understand why the standard Unity Scroll View still generate using a Mask instead of a RectMask2D.

[FreeThinker]

On 3/25/2018 at 6:58 PM, linuxgurugamer said:

One thing is to eliminate all the "foreach" statements, replace them with "for (int i = 0; i < xxx.Count; i++) {var s = xxx; }  

 

Does this hold for all foreach loops? what about the link ForEach?

[linuxgurugamer]

7 minutes ago, FreeThinker said:

Does this hold for all foreach loops? what about the link ForEach?

In general, yes, although I believe that depending on the compiler being used and the exact statement, it may change.

[DMagic]

1 hour ago, FreeThinker said:

Does this hold for all foreach loops? what about the link ForEach?

My understanding is that this has been improved or fixed in the latest versions of Unity (5.5 and up), but that it might be dependent on what type of data is in the collection and what type of collection. It's hard to find a straight answer.

From what I can gather it sounds like foreach loops on a List<T> with value-type data (primitives like float, vector3, etc.., or any other struct) are fine, but looping over reference-type data might still allocate garbage. Here is some data about different collection types running a foreach loop: https://jacksondunstan.com/articles/3805. You can see that a List<T> in Unity 5.6 generates no garbage, but that is only tested with value-type data. You can also see that some collection types only generate garbage the first time they run (though I assume this only applies when the collection is not being modified in any way between runs).

I guess you could run some simple test code in the Unity Editor, using stand-ins for whatever type of loop you are testing, then check for garbage allocations in the profiler. 

The easiest thing is probably just to avoid foreach loops in anything that runs continuously.

[FreeThinker]

48 minutes ago, DMagic said:

My understanding is that this has been improved or fixed in the latest versions of Unity (5.5 and up), but that it might be dependent on what type of data is in the collection and what type of collection. It's hard to find a straight answer.

From what I can gather it sounds like foreach loops on a List<T> with value-type data (primitives like float, vector3, etc.., or any other struct) are fine, but looping over reference-type data might still allocate garbage. Here is some data about different collection types running a foreach loop: https://jacksondunstan.com/articles/3805. You can see that a List<T> in Unity 5.6 generates no garbage, but that is only tested with value-type data. You can also see that some collection types only generate garbage the first time they run (though I assume this only applies when the collection is not being modified in any way between runs).

I guess you could run some simple test code in the Unity Editor, using stand-ins for whatever type of loop you are testing, then check for garbage allocations in the profiler. 

The easiest thing is probably just to avoid foreach loops in anything that runs continuously.

Thanks, and what about temporary variables? I often already made local variable globally private for debugging purposes. Especially for methods that are called every frame, is it still advantageous to minimize temporary variable by recycling them to minimize garbage?

Edited April 26, 2018 by FreeThinker

[sarbian]

Ok, stop right there. Move your hands away from the code and find an article about how C# works and read and reread the chapter about object, reference type, value type and allocation.

Do not try to optimize anything until you understand that.

[FreeThinker]

22 minutes ago, sarbian said:

Ok, stop right there. Move your hands away from the code and find an article about how C# works and read and reread the chapter about object, reference type, value type and allocation.

Do not try to optimize anything until you understand that.

Ok so values types disappear from the stack when poped while reference types located on the heap could lead to excessive garbage

[Warezcrawler]

5 hours ago, sarbian said:

Ok, stop right there. Move your hands away from the code and find an article about how C# works and read and reread the chapter about object, reference type, value type and allocation.

Do not try to optimize anything until you understand that.

I’m not sue I agree. More than a few of us are amatuers. If you ask me, should try to optimize anyways.  This topic is supposed to help some of us get some guidelines for what not to do. 

So please sarbian, if you have some point, then please share it. 

Do not expect me to ever become a c# professional. But I do what to make a mod that does not hit performance in my/our gaming experience. 

If the price is to do premsture optimization, so be it. 

[xEvilReeperx]

8 hours ago, Warezcrawler said:

I’m not sue I agree. More than a few of us are amatuers. If you ask me, should try to optimize anyways.  This topic is supposed to help some of us get some guidelines for what not to do.  

You're going to have a hard time writing code that doesn't generate unnecessary garbage if you don't understand the mechanisms that create it. Use the profiler to find problem spots.

If you happen to use VS and have Resharper, the Heap Allocations Viewer extension is very useful

[sarbian]

13 hours ago, Warezcrawler said:

Do not expect me to ever become a c# professional.

I asked to read about the very base of the langage. That has nothing to do with being a C# pro and has everything to do with being a C# anything.

@xEvilReeperx is spot on. If you do not understand what you are doing you may very well make the problem worse.

[Warezcrawler]

5 hours ago, xEvilReeperx said:

You're going to have a hard time writing code that doesn't generate unnecessary garbage if you don't understand the mechanisms that create it.

True, and pretty much the point of this thread... But I can still try to optimize even when my understanding is imperfect.

5 hours ago, xEvilReeperx said:

Use the profiler to find problem spots.

How do you profile a KSP plugin like partmodule? I've seen it in unity videos, but how can we access it?

5 hours ago, xEvilReeperx said:

If you happen to use VS and have Resharper, the Heap Allocations Viewer extension is very useful

I've guessing resharper is an extension for VS... Is it free? I don't have it (yet). Do they do the job in KSP modding?

2 minutes ago, sarbian said:

everything to do with being a C# anything.

Ok... You probably have a point (you usually do)... I'm read a lot C# sharp, but not sure exactly what you are pointing me at, so can you maybe help with a link as a starting point?

4 minutes ago, sarbian said:

not understand what you are doing you may very well make the problem worse.

True, but I will still try my best, and I'm learning at the same time. This is also the point when I ask for some guidelines. Ex. avoid "foreach" is a simple guideline that helps and as far as I can see does not create a problem. And there are many more help points in this thread.

I found this link that really told me a lot

https://unity3d.com/learn/tutorials/topics/performance-optimization/optimizing-garbage-collection-unity-games

[genericeventhandler]

On 3/25/2018 at 6:58 PM, linuxgurugamer said:

One thing is to eliminate all the "foreach" statements, replace them with "for (int i = 0; i < xxx.Count; i++) {var s = xxx; }

  

 As a side note,  it's better to do your count once outside of the loop as well as Initialize your loop variable, if you are looking to produce the cleanest fastest way to loop. Like this:

var count = xxx.Count;
int loop = 0;

for(loop = 0; loop < count ; loop++){
  
}

Geh

 

 

[sarbian]

50 minutes ago, genericeventhandler said:

int loop = 0;

Explain how this is an optimization.

[DMagic]

6 hours ago, genericeventhandler said:

 As a side note,  it's better to do your count once outside of the loop as well as Initialize your loop variable, if you are looking to produce the cleanest fastest way to loop. Like this:

My understanding was that it recalculates the max value for every iteration of the loop, I don't know if that's actually true (Edit: random googling shows probably no difference, and there are compiler optimizations and caching that affect this, too). So if you do something silly like List.Count() instead of List.Count it would have some kind of impact.

Most of the time I just run the loops backwards anyway [ for( i = list.Count; i > 0; i++) ], which also makes it safer to adjust the collection while looping through it.

8 hours ago, Warezcrawler said:

How do you profile a KSP plugin like partmodule? I've seen it in unity videos, but how can we access it?

You can always do things like using a stopwatch to check for how long something takes. And you can use GCMonitor or MemGraph to check for garbage allocation, but it's hard to find out any kind of fine-grained information that way. Those also requiring actually running KSP, which is a pain.

If you run code in the Unity Editor then you can activate the profiler and find out exactly how long everything takes (though the profiler itself has an effect on performance) and how much garbage it allocates. But that is obviously tricky unless you have some specific code to test that doesn't require KSP at all.

Edited April 27, 2018 by DMagic

[Lisias]

On 4/27/2018 at 3:30 PM, DMagic said:

Most of the time I just run the loops backwards anyway [ for( i = list.Count; i > 0; i++) ], which also makes it safer to adjust the collection while looping through it.

There's a one more little catch. prefixed and postfixed "++' and '--' has different colateral effects, and depending on what you are doing, it's better to use one or another.

The postfix (i++) creates a temporary copy of the data (as it returns the previous value before incrementing). The prefix (++i) returns the data itself, as the previous value is irrelevant and there's no need to preserve it for using later.

This is specially relevant when your objects overloads the "++" (assuming you follow the standard to the letter).

So,  [ for( int i = list.Count; i >= 0; --i) ] is even better.

Edited April 29, 2018 by Lisias
I made a mistake, used the increment and not the decrement! Fixed (see below)

[xEvilReeperx]

8 hours ago, Warezcrawler said:

How do you profile a KSP plugin like partmodule? I've seen it in unity videos, but how can we access it?

Follow the same instructions to get debugging working, and then use Unity's profiler with KSP

[NavyFish]

On 4/27/2018 at 12:16 PM, Lisias said:

There's a one more little catch. prefixed and postfixed "++' and '--' has different colateral effects, and depending on what you are doing, it's better to use one or another.

The postfix (i++) creates a temporary copy of the data (as it returns the previous value before incrementing). The prefix (++i) returns the data itself, as the previous value is irrelevant and there's no need to preserve it for using later.

This is specially relevant when your objects overloads the "++" (assuming you follow the standard to the letter).

So,  [ for( int i = list.Count; i >= 0; ++i) ] is even better.

that's an infinite loop.  you all probably meant to decrement i.

also.. micro optimizations such as this are almost pointless. the hogs are going to be per-frame object allocations (and the subsequent GC) and improper use of unity's component system, like searching for a component by tag every frame instead of caching the result, etc.  

[Warezcrawler]

In all my reading about the GC, I've come across a topic I can't seem find the optimal structure for - and maybe there is nothing to optimize. Consider the following in a Update() scenario.

Creating new objects and destroying them creates garbage. i.e. 

List<object> myobjectlist = new List<object>();

This is often better just to create the list outside the update, and maybe do a clear ind order to reduce garbage created.

Then I have the thing I'm most puzzled about, that is when I create an object that references an already created game object.

somegameobject cacheModuleEngines = somegameobject.instance;

Now I have a simple reference for that game object, which the GC have to analyse when doing GC. However, I there anything I can do to optimize? Would setting it to null help anything? Is it worth it?

I would normally not care too much here, but with the excessive GC in KSP when modded, I'm inclined to do every optimization I can think of knowing that the main issue comes from elsewhere :o(.

But again, what are your thoughts on this. Is this simple too insignificant to think of, or is there anything to do?

[Lisias]

5 hours ago, NavyFish said:

that's an infinite loop.  you all probably meant to decrement i.  

Yes, I meant a decrement! :-) Thanks!

But not, it's not an infinite loop - since I used int, and not unsigned int, sooner or later it would happen a integer overflow, the value would be "incremented" to (-(MAXINT+1)) and the loop would end after raising a really awful amount of exceptions. :-)

5 hours ago, NavyFish said:

also.. micro optimizations such as this are almost pointless.  

This is not an optimization. This is the way it's intended to be used. Using the postfixed operator in these cases is an anti-pattern, just it.

And when you are dealing with objects that properly overloaded the ++ and -- operators, you end up flooding the GC with garbage - so, far, far away from being pointless.

5 hours ago, NavyFish said:

 the hogs are going to be per-frame object allocations (and the subsequent GC) and improper use of unity's component system, like searching for a component by tag every frame instead of caching the result, etc.  

Using things the right away helps to prevent such hogs, be it the Unity, be it the language features.

Not fixing a small error because you are doing worst errors on your code just pile up the errors.

[Lisias]

On 4/27/2018 at 10:04 AM, sarbian said:

Explain how this is an optimization.

In the beginning of time, when ANSI C compilers were a lot dumber, there was very little optimisations available while code generating. So you had to do it yourself.

Knowing that for (int i = 0; i < n; ++i) { something(i); } is kind of a shortcut to { int i; for (i = 0; o < n; ++i) { something(i); } } , and knowing that scoped variables are allocated on the thread's stack at the declaration and the stack is cleaned on the end of the scope, you used to get significantly speed gains (by preventing such code to be generated) by using this stunt.

Even nowadays it's a issue. But nowadays the compilers automatically detects these situations, reusing the stackspace when successive scopes declares variables and/or delaying the stack cleaning, doing it out of loops. Google for "Variable coalescence" or "variable graph coloring".

Old school guys from that era that didn't got an update on his training usually tend to stick to that practice,

Some embedded toolchains, old but still widely used, still needs it - but this is niche specific.

Edited May 5, 2018 by Lisias
yet more typos.