Jump to content

Devnote Tuesday: Smashing Buttons


SQUAD

Recommended Posts

15 hours ago, NathanKell said:

We're converting them all to for loops.

The issue is that in Unity's mono, foreach creates garbage. And linq more or less _has_ to create garbage AIUI, and it does it in spades in Unity.

This sounds unhealthy. Have you considered overriding the Enumerator (which seems to create this mess) with a bug-free implementation?

I can understand why you may want to avoid LINQ expressions, late evaluation can easily blow up in your face. But for-each loops are usually much, much safer and sometimes even faster than regular for loops. Rewriting half your code base because of one bug sounds not very reasonable.

Link to comment
Share on other sites

2 hours ago, linuxgurugamer said:

I wasn't aware of the issues with "foreach".  I like it because it makes the code simpler, but if it contributes to the lagging, I'll be working on removing them from all my mods.  gonna take some time though

Here's a pretty good article which compares a few different ways of doing things, and the performance implications in Unity/Mono.  Of particular interest is this summary:

// const SIZE = 16 * 1024 * 1024;
// array is an int[]
// list is a List<int>

1a. for (int i = 0; i < SIZE; i++) { x += array[i]; }
1b. for (int i = 0; i < SIZE; i++) { x += list[i]; }
2a. foreach (int val in array) { x += val; }
2b. foreach (int val in list) { x += val; }
 3. x = list.Sum(); // linq extension

                              time   memory
1a. for loop over array .... 35 ms .... 0 B
1b. for loop over list ..... 62 ms .... 0 B
2a. foreach over array ..... 35 ms .... 0 B
2b. foreach over list ..... 120 ms ... 24 B
 3. linq sum() ............ 271 ms ... 24 B

LINQ is the best from a code readability and maintainability perspective.  But the cost is just too darn high.

Link to comment
Share on other sites

1 hour ago, marce said:

Not the compiler, the runtime. And Unity is to blame (at least for a big part) because they keep using this stone-age version of Mono instead of paying Miguel (now that would be MS I suppose) for a new one.
Anyway, I think I read something about Unity working on a .NET Native like project to improve performance.

To add to what @Padishar said : Unity uses mono on many platforms. All the console/phone platform uses the mono code/binaries that Xamarin wrote. Those console/phone version were not open source and free, and you needed a licence from Xamarin to use them. Unity wanted to upgrade their runtimes but Xamarin asked too much money (Unity's version here, not mine) for the new license. 

To avoid paying for those high fee Unity developed their IL2CPP system (that also allow them to follow the "no interpreted code" Apple guideline). 

Now Microsoft has released a open source .NET implementation (CoreCRL) and bought Xamarin. The whole Xamarin code base has been opened and the Mono also include a lot of the MS code (that is way better in some aspect). So no new licence needed.

 

Unity now plan to upgrade their .NET runtimes. It may be that the Desktop will run on CoreCRL and all the other system in IL2PP in the future.

 

Link to comment
Share on other sites

1 hour ago, linuxgurugamer said:

Bad implementaiton in the c# compiler, not unity.

and instead of "correct", I would say "preferred" in terms of maintainability and readability.

That is an excellent explanation, thank you for putting it so clearly

Are you looking at the KSP.log or the output_log.txt file?

KSP.log doesn't show everything.  Check the KSP_Data/output_log.txt (for 32 bit) or KSP_x64_Data/output_log.txt (for 64 bit)

I don't think it is recording anything in that log ether. it looks like it normally does if it doesn't crash. it seems unity is totaly crashing. i will keep investigating.it could be a specific mod.

Link to comment
Share on other sites

11 minutes ago, nightingale said:

Here's a pretty good article which compares a few different ways of doing things, and the performance implications in Unity/Mono.  Of particular interest is this summary:


2a. foreach over array ..... 35 ms .... 0 B

LINQ is the best from a code readability and maintainability perspective.  But the cost is just too darn high.

Just to avoid to confuse anyone : foreach over an array is the only foreach case that does not use any memory.

Link to comment
Share on other sites

3 minutes ago, Monger said:

This sounds unhealthy. Have you considered overriding the Enumerator (which seems to create this mess) with a bug-free implementation?

That isn't possible.  The bug isn't in the enumerator, it is the Mono runtime that incorrectly allocates the struct returned from GetEnumerator on the heap.

5 minutes ago, Monger said:

I can understand why you may want to avoid LINQ expressions, late evaluation can easily blow up in your face. But for-each loops are usually much, much safer and sometimes even faster than regular for loops.

It isn't anything to do with late evaluation, that part of LINQ is fine and can significantly improve some things like database access.  The real problem is that various LINQ constructs end up creating temporary arrays of results (on the heap regardless of runtime) and lots of enumerator objects (also on the heap in Unity's Mono) resulting in large quantities of garbage.  The editor parts list was doing a LINQ query to get the parts in the current category on every frame resulting in ~5 MB/s of garbage (at 60 fps, more at higher frame rates) when showing the largest stock category.  This was replaced in 1.1.3 which reduced the garbage creation to ~500 KB/s.

Just now, sarbian said:

Just to avoid to confuse anyone : foreach over an array is the only foreach case that does not use any memory.

Indeed, this is because of a specific optimisation in Mono that converts foreach over an array into a plain for loop.  That's why the time is also the same as the 1a case.

Link to comment
Share on other sites

15 hours ago, Foxster said:

OK, I've got where you are coming from but the basic game with the solar system and the craft parts hasn't changed much in quite a while, apart from bits and pieces. I really think it's time to get creative with the amazing base platform you now have and take the game forward, rather than constantly polishing what you have.

Otherwise you know someone is going to steal the basic idea and make a more engaging product, leading to the demise of KSP.   

Can you think of anything that should be added to the game that is not already available in a mod?

The only thing you cannot add via a mod is a rock-solid foundation, and that is what they are working on now.

Once that is in place, it will be quicker and easier to add new features and those that use mods will be able to support even more mods.

 

15 hours ago, Glaran K'erman said:

Great DevNotes! Just one thing missing, the poem.

01010010 01101111 01110011 01100101 01110011 01100001 01110010 01100101 01110010 01100101 01100100 01010110 01101001 01101100 01101111 01100101 01110100 01110011 01100001 01110010 01100101 01100010 01101100 01110101 01100101 01001001 01100100 01101111 01101110 00100111 01110100 01110101 01101110 01100100 01100101 01110010 01110011 01110100 01100001 01101110 01100100 01100011 01101111 01101101 01110000 01110101 01110100 01100101 01110010 01110011 01100011 01101001 01100101 01101110 01100011 01100101 00101100 01100010 01110101 01110100 01101111 01100010 01110110 01101001 01101111 01110101 01110011 01101100 01111001 01111001 01101111 01110101 01100100 01101111 00100001

Hey people who code, how does it feel not understanding anything you just read!

In this day and age there are only 10 reasons not to understand something you are reading online:

01: the content is actually meaningless giberish

10: you do not consider the content worth the effort to decipher using the tools available to you.

Personally, my curiosity puts a pretty high bar on how much effort I am willing to go through to understand something.

 

13 hours ago, kiwi1960 said:

By restarts..... I mean having to restart my entire game from scratch... as is always the case, it seems.

My 1.1 game has transferred just fine to 1.1.3 (except for the modular fuel tanks mod not being updated last time I checked, leaving me with hundreds of units of oxidizer on a NERV powered ship, but I expect that will be fixed in time, or else I'll just recover it and launch a new one using different tanks)

Link to comment
Share on other sites

33 minutes ago, Monger said:

I can understand why you may want to avoid LINQ expressions, late evaluation can easily blow up in your face. But for-each loops are usually much, much safer and sometimes even faster than regular for loops. Rewriting half your code base because of one bug sounds not very reasonable.

This would be fine with a modern GC that allocate those short lived object in a specific memory zone to clean them up faster later. But we do not have it so even a 24 byte allocation manage to hurt performance. The article nightingale linked is a good read on Unity memory :)

Link to comment
Share on other sites

4 hours ago, Monger said:

But for-each loops are usually much, much safer and sometimes even faster than regular for loops.

The only way in which they can be safer is that they don't allow you to mess with the index in the loop body which is usually more of a hindrance than a help.  Can you provide an example of a foreach loop being faster than a (correctly written) for loop?  Given what the compiler and runtime does, I can't see any way that this could ever be the case.

The real killer with foreach is when you nest it, from a little further down that article listed:

Quote

Let’s try something else, this time the same number of elements only arranged in a 2D array, of 4K arrays or lists each 4K elements long, using nested for loops vs nested foreach loops:


                                      time    memory
1a. for loops over array[][] ......  35 ms ..... 0 B
1b. for loops over list<list<int>> . 60 ms ..... 0 B
2a. foreach on array[][] ........... 35 ms ..... 0 B
2b. foreach on list<list<int>> .... 120 ms .... 96 KB <-- 

Here, there are 4097 temporary enumerator objects being allocated, one for the outer loop and one for each time the inner loop is run.  It is easy to write that sort of code in KSP where there is a List<Vessel>, each vessel contains a List<Part> and Part contains several other lists.  If you're not careful you can write a function that causes many megabytes of garbage every time it's run...

Edited by Padishar
typo
Link to comment
Share on other sites

shouldnt we add this discussion to the Add-On-Forum and try to make sure as much as possible modders are going to rework their mods for that? => Sticky, red highlighted blinking font?

Most people suffering with the Garbage Problems use tons of mods and it might take much pressure from the devs to "optimize the game", when all mods would be optimized as well..
The Community would therefor add to the Optimization of the game as well as to the content as it is doing today.

Edited by Speadge
Link to comment
Share on other sites

23 minutes ago, Padishar said:

That isn't possible.  The bug isn't in the enumerator, it is the Mono runtime that incorrectly allocates the struct returned from GetEnumerator on the heap.

The bug's in the compiler used, not the runtime itself. The following, built in VS, doesn't generate any garbage in Update() because the emitted IL doesn't box the value type iterator before calling dispose on it like it would when built from Unity

[KSPAddon(KSPAddon.Startup.MainMenu, false)]
public class ExampleScript : MonoBehaviour
{
    private readonly List<List<int>> _list = new List<List<int>>();

    private void Start()
    {
        for (int i = 0; i < 4096; ++i)
        {
            _list.Add(new List<int>(Enumerable.Repeat(1, 4096)));
        }
    }

    public void Update()
    {
        int i = 0;

        foreach (var list in _list)
        {
            foreach (var item in list)
                i += item;
        }
    }
}

 

 

Link to comment
Share on other sites

I'm an amateur vb.net programmer, and I still use For Each instead of For i wherever I can. It's slightly more readable, and the applications I make don't need to be optimized much because they serve such basic functions. I could see it being a huge deal in a video game, though, when you're trying to do 60 frames per second. And that said, the readability of this:

 

For i = 0 to MyList.Count - 1

DoStuff(MyList.item(i))

i += 1

Loop

 

...isn't that much harder to read than this:

 

For Each MyItem as fooType in MyList

DoStuff(MyItem)

Loop

 

It's good to have the options and know the strengths and drawback of each, though. Thanks for the info!

Link to comment
Share on other sites

13 minutes ago, xEvilReeperx said:

The bug's in the compiler used, not the runtime itself. The following, built in VS, doesn't generate any garbage in Update() because the emitted IL doesn't box the value type iterator before calling dispose on it like it would when built from Unity

This is true, the problem is supposedly in the compiler shipped with Unity (though I have seen a few odd behaviours that seem to contradict this).  The replacement of foreach loops in mods should not be necessary from a garbage standpoint (assuming the object in question does return a struct from GetEnumerator and you build your mod with a non-broken compiler) though it will still make a small difference to the performance.  KSP does use the Unity compiler so it does suffer from this and does benefit from removing foreach.  I am a bit surprised that they didn't investigate building their assemblies with the MS compiler (or a newer Mono one?) instead of replacing all the foreach loops, though they would still need to clean up a lot of LINQ stuff and other sources of garbage.

Link to comment
Share on other sites

17 hours ago, Glaran K'erman said:

Great DevNotes! Just one thing missing, the poem.

01010010 01101111 01110011 01100101 01110011 01100001 01110010 01100101 01110010 01100101 01100100 01010110 01101001 01101100 01101111 01100101 01110100 01110011 01100001 01110010 01100101 01100010 01101100 01110101 01100101 01001001 01100100 01101111 01101110 00100111 01110100 01110101 01101110 01100100 01100101 01110010 01110011 01110100 01100001 01101110 01100100 01100011 01101111 01101101 01110000 01110101 01110100 01100101 01110010 01110011 01100011 01101001 01100101 01101110 01100011 01100101 00101100 01100010 01110101 01110100 01101111 01100010 01110110 01101001 01101111 01110101 01110011 01101100 01111001 01111001 01101111 01110101 01100100 01101111 00100001

Hey people who code, how does it feel not understanding anything you just read!

54 68 65 49 6e 74 65 72 6e 65 74 49 73 41 6d 61 7a 69 6e 67

Link to comment
Share on other sites

18 hours ago, Glaran K'erman said:

Great DevNotes! Just one thing missing, the poem.

01010010 01101111 01110011 01100101 01110011 01100001 01110010 01100101 01110010 01100101 01100100 01010110 01101001 01101100 01101111 01100101 01110100 01110011 01100001 01110010 01100101 01100010 01101100 01110101 01100101 01001001 01100100 01101111 01101110 00100111 01110100 01110101 01101110 01100100 01100101 01110010 01110011 01110100 01100001 01101110 01100100 01100011 01101111 01101101 01110000 01110101 01110100 01100101 01110010 01110011 01100011 01101001 01100101 01101110 01100011 01100101 00101100 01100010 01110101 01110100 01101111 01100010 01110110 01101001 01101111 01110101 01110011 01101100 01111001 01111001 01101111 01110101 01100100 01101111 00100001

Hey people who code, how does it feel not understanding anything you just read!

Happens to me all the time

2 hours ago, sarbian said:

Just to avoid to confuse anyone : foreach over an array is the only foreach case that does not use any memory.

Hmmm.  So the following two examples are essentially identical:

string[] sar = new string[1000];

foreach (string s in sar)

vs

for (int i = sar.Count - 1; i >= 0; i++)

This will help, I do a lot of foreach's over string arrays

Edited by linuxgurugamer
Link to comment
Share on other sites

33 minutes ago, linuxgurugamer said:

Hmmm.  So the following two examples are essentially identical:

Well, in terms of speed and memory use, yes.  The second one scans through them in the reverse order which could have a significant effect on what the code actually does.

Note @xEvilReeperx's comment above (and my confirmation) that foreach only gives  garbage problems when the code is compiled with the compiler that is shipped as part of Unity.  If you use VisualStudio (or, probably, a recent version of MonoDevelop) then you don't really need to change foreach at all.

Link to comment
Share on other sites

3 hours ago, sarbian said:

[...]

Unity now plan to upgrade their .NET runtimes. It may be that the Desktop will run on CoreCRL and all the other system in IL2PP in the future.

Good news! Thanks for the info.

Link to comment
Share on other sites

43 minutes ago, Red Iron Crown said:

It violates rule 2.4b and makes the thread harder to read.

Same could be said for all the coding discussion. I know that it's on-topic for this particular dev notes, just saying that the depth and specifics of that probably deserves it's own thread in Add-on Discussions. 

(And yes, I know "if you don't want to read about the code, ignore those posts." I'm just saying that the same could be said about the binary stuff.) 

Link to comment
Share on other sites

1 minute ago, FullMetalMachinist said:

Same could be said for all the coding discussion. I know that it's on-topic for this particular dev notes, just saying that the depth and specifics of that probably deserves it's own thread in Add-on Discussions. 

(And yes, I know "if you don't want to read about the code, ignore those posts." I'm just saying that the same could be said about the binary stuff.) 

The binary stuff is deliberately obfuscated, which is what 2.4b is meant to prevent.

The coding talk is not deliberately obfuscated, it's written as clearly as it can be.

If you wish to discuss it further please take it to PM with me or another moderator, as it's offtopic here.

Link to comment
Share on other sites

On my part, if 1.2 brings absolutly nothing else than making everything work as it should, i'll call it the best KSP update ever.

Take ALL the time that will be needed, no point rushing release like 1.1if it is not 100% ready.

Cool new stuffs and moar boosters can wait for the next update, as long as everything now work as intended!

Link to comment
Share on other sites

Now for a poem people can read,:

1.1.3 is done,
Now we're on to 1.2.

Spring cleaning arrives,
Everybody's getting ready for new Unity

Talking in devspeak,
Thinking like devs think.

Building foundations by tearing down walls,
Breaking linq's to take out the garbage.

Asteroid day's being updated, not a moment too soon,
Antennae are comm-ing, not long, I assume.

 

 

 

Link to comment
Share on other sites

2 hours ago, Padishar said:

Well, in terms of speed and memory use, yes.  The second one scans through them in the reverse order which could have a significant effect on what the code actually does.

Note @xEvilReeperx's comment above (and my confirmation) that foreach only gives  garbage problems when the code is compiled with the compiler that is shipped as part of Unity.  If you use VisualStudio (or, probably, a recent version of MonoDevelop) then you don't really need to change foreach at all.

I was referring to speed & memory use, thanks.

But, it seems that I don't have to do anything, since I'm using VisualStudio, the compiler is already fixed.

cool.

Link to comment
Share on other sites

This thread is quite old. Please consider starting a new thread rather than reviving this one.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...