[Csantucci]

Dennis,
as no one else was tackling the problem, I have tried applying your patch, as I was continuing to get crashes. Thank you very much for your proposal! What I found out is that removing the distinction between standard and frequent update sounds, as you did, significantly increases CPU load for the sound process and decreases FPS. So I tried re-introducing the distinction. Even so, CPU load is still higher than before your patch, however effect on FPS seems smaller, at least in my test case. Before committing anything, I attach here the patch I am using, and would like you to have a look and to give an opinion, as not everything you have implemented is clear to me, so maybe there is a fault in my changes; if there is a suggestion to further reduce CPU load towards the original value, it is welcome.

 SoundProcess_mio.cs.patch.zip (2.24K)
Number of downloads: 148

[dennisat]

 Csantucci, on 02 January 2015 - 08:26 AM, said:

I attach here the patch I am using, and would like you to have a look and to give an opinion, as not everything you have implemented is clear to me,

I'll look at it as soon as possible. I've refined my original patch somewhat because it was a first attempt and I discovered some poor code. It damaged nothing and did stop the crashes but some of the things I thought I was doing didn't work! That may be why your resource usage increased. Also, the statistics are not necessary to the function; I was just trying to get an idea of how long it took to process the sound sources in some heavy activities.

Dennis

[dennisat]

 dennisat, on 02 January 2015 - 11:11 AM, said:

I'll look at it as soon as possible.

You have got the essence of my idea Carlo, but my restart after interrupt logic was wrong. In fact, it just started from the beginning of the update again instead of approximately where it had been interrupted. I was generating increased resource usage although the time code was locked was decreased and the crashes avoided. I am being called to dinner right now, I'll produce a modification of your patch for you to review tomorrow, I hope.

Dennis

[dennisat]

 dennisat, on 02 January 2015 - 12:25 PM, said:

.. I'll produce a modification of your patch for you to review tomorrow, I hope..

Hi Carlo,

I've adjusted your patch to correct my error with the restart and also altered a few comments. I notice you've put all the statistics coding into debug blocks. Because of some poor coding on my part, the statistics produced were not reliable! I suggest you get rid of them or rework them to produce something meaningful if you're going to use this patch.

Using lock was the original method of protection of the data and it was replaced by Interlocked Compare/Exchange which unfortunately does not seem to protect the Lists that the Dictionary points to.

To get rid of the random errors I therefore reverted to using lock. This has major disadvantages, especially with the sound processing, because I've measured some prolonged periods on sound dense activities (more than 5 minutes) when it was taking more than 300ms to process the sound updates. During this 300ms, which is an eon in modern computing, adds and deletes to the sound sources could not be processed. Since loading tiles can cause a lot of sound source additions and deletions, I thought this could be a contributory cause of sound stuttering. I therefore decided to try allowing the sound source update to be interrupted when additions and deletions were required. Subjectively, stuttering seems to have decreased but I've got no hard data to support this - how do you measure degree of stutter?

I've used Interlocked Compare/Exchange to manage an interrupt switch which, in my old IBM mainframe days, used to be called a "spin lock". You can probably see why. Event signalling may be a better way of doing this in C# but my understanding of C# hasn't got that far yet.

Dennis

 SoundProcess_mio_1.cs.patch.zip (2.44K)
Number of downloads: 151

[Csantucci]

Dennis,
thank you for your new patch. I tested it, and at a first test I got more or less the same performances as with the previous patch version, that is sound process requires about 20-25% more CPU load than without patch (that is if e.g. it was 40% before patch, it is 50% now); however this seems not to have bad effects on the FPS; on the contrary, in a specific medium-load case, FPS seem higher with patch than without patch (and also higher than with the previous version of the patch)! Before committing I would like to see if there is some pertinent comment on this.

[dennisat]

 Csantucci, on 03 January 2015 - 08:21 AM, said:

.. sound process requires about 20-25% more CPU load than without patch...

I think you can bring that down by restoring sleeptime to a fixed 50ms. This may be better for processors with less than 4 cores. A variable sleeptime can take better advantage of a 4 (or more) core processor because it will not matter if the sound thread runs up to 100%. Other threads will hardly be affected and may even benefit by getting sound processes on their behalf through quicker. On a 2 core though, the sound process will be sharing with other processes and they may be degraded if it is allowed too much resource.

Dennis

[Csantucci]

I've uploaded your patch in release 2755 restoring the fixed 50 ms. I've only commented the lines that provide variable sleep time, so that in a future time they could be considered again for insertion.
Dennis, thank you again for your fix.

[James Ross]

I can't believe you've reintroduced that terrible lock in this code. :good:

Expect me to remove it again in the future.

[dennisat]

 James Ross, on 03 January 2015 - 12:17 PM, said:

I can't believe you've reintroduced that terrible lock in this code. :good:

Expect me to remove it again in the future.

You might well find a better way. However, I run sound laden routes and activities (like MEP and Dorset Coast) and the crashes were incessant - it was as bad as MSTS. I did spend quite a while trying to get the Interlocked Compare/Exchange method to work but whatever variety I tried, it did not seem to protect (ie copy) the lists that the dictionary pointed to. I was starting to think that you'd have to individually copy these lists but that would be barmy because you'd probably have to use lock to copy them all in sync! In the end I gave up and took the easy way out. The update, the major bottleneck, is now interruptible so I don't think there'll be much advantage in getting the Interlocked Compare/Exchange method to work.

I suspect you'll take that as a challenge! I certainly would.

Best of luck
Dennis

[Csantucci]

As always, better solutions are welcome. However, after about two months of crashes, this at least avoids the crashes.