locked
Async and lock don't mix, but how to do *this*?

    Question

  • I have an audio application that has dedicated background threads to process audio. I'm running into endless problems because the async/await pattern is now being forced down into everything, even in places where it does not belong. Performance tests have shown that async/await adds horrific amounts of latency to calls which interferes with realtime audio processing.

    For starters, I would like someone to show me how to do the following, given these requirements:

    1. Multiple background threads can request data from a shared cache in random access fashion, one byte at a time.
    2. If the data is not in the cache, the requesting thread reads it using Stream.ReadAsync from a file and saves it into the cache.
    3. All do's and dont's of async/await must be followed.

    To clarify, here is the non-async version:

    private byte GetData(int index)
    {
        lock (cacheLock)
        {
            if (!IsDataInCache(index))
            {
                ReadDataIntoCache(index);  // Sync
            }
     
            return GetDataFromCache(index);
        }
    }

    And here is the async version that needs fixing:

    private byte GetData(int index)
    {
        lock (cacheLock)
        {
            if (!IsDataInCache(index))
            {
                await ReadDataIntoCacheAsync(index);
            }
     
            return GetDataFromCache(index);
        }
    }

    I know the above async version won't compile because you can't have async/await withing a lock, but I would like to know how to fix it.

    Thanks.

    EDIT: BTW, I'm aware that one can simply add Wait at the end instead of await, however this is supposedly one of the dont's, and if you need to do that your call latency is already down the toilet.

    • Edited by BitFlipper Monday, June 23, 2014 3:35 AM
    Monday, June 23, 2014 12:29 AM

All replies

  • Use the Wait and be done with asking hypothetical questions with no answer. 

    Matt Small - Microsoft Escalation Engineer - Forum Moderator
    If my reply answers your question, please mark this post as answered.

    NOTE: If I ask for code, please provide something that I can drop directly into a project and run (including XAML), or an actual application project. I'm trying to help a lot of people, so I don't have time to figure out weird snippets with undefined objects and unknown namespaces.

    Monday, June 23, 2014 1:40 PM
    Moderator
  • Use the Wait and be done with asking hypothetical questions with no answer.

    That doesn't answer the question since using Wait is considered Doing It Wrong(TM). We need to decide what is and isn't the right way to do things and not flip flop all over the place.

    And as far as performance, see this:

    private async void Test()
    {
        Stopwatch sw = null;
        var result = 0L;
        var iterations = 1000000;
    
        sw = Stopwatch.StartNew();
    
        for (var idx = 0; idx < iterations; idx++)
            result += DoSomething();
    
        var syncTime = sw.Elapsed;
    
        result = 0;
        sw = Stopwatch.StartNew();
    
        for (var idx = 0; idx < iterations; idx++)
            result += await DoSomethingAsync();
    
        var async1Time = sw.Elapsed;
    
        result = 0;
        sw = Stopwatch.StartNew();
    
        for (var idx = 0; idx < iterations; idx++)
        {
            var task = DoSomethingAsync();
    
            task.Wait();
            result += task.Result;
        }
    
        var async2Time = sw.Elapsed;
    
        Debug.WriteLine("Sync:   " + syncTime.ToString() + " which is " + (syncTime.TotalSeconds / iterations * 1000.0).ToString("F6") + " ms per op");
        Debug.WriteLine("Async1: " + async1Time.ToString() + " which is " + (async1Time.TotalSeconds / iterations * 1000.0).ToString("F6") + " ms per op");
        Debug.WriteLine("Async2: " + async2Time.ToString() + " which is " + (async2Time.TotalSeconds / iterations * 1000.0).ToString("F6") + " ms per op");
        Debug.WriteLine("");
        Debug.WriteLine("Async1 is " + (async1Time.TotalSeconds / syncTime.TotalSeconds).ToString("F1") + " times slower");
        Debug.WriteLine("Async2 is " + (async2Time.TotalSeconds / syncTime.TotalSeconds).ToString("F1") + " times slower");
    }
    
    protected virtual int DoSomething()
    {
        return 42;
    }
    
    protected async virtual Task<int> DoSomethingAsync()
    {
        await Task.Yield();
        return 42;
    }

    And the results:

    Debug:
    Sync:    00:00:00.0204956 which is 0.000020 ms per op
    Async1: 00:02:41.7834629 which is 0.161783 ms per op
    Async2: 00:02:39.1836125 which is 0.159184 ms per op

    Async1 is 7893.6 times slower
    Async2 is 7766.7 times slower


    Release:
    Sync:    00:00:00.0089678 which is 0.000009 ms per op
    Async1: 00:02:37.7255046 which is 0.157726 ms per op
    Async2: 00:02:22.9098871 which is 0.142910 ms per op

    Async1 is 17588.0 times slower
    Async2 is 15935.9 times slower


    EDIT: BTW, there is nothing hypothetical about the question. I have a real problem and I need a real solution. I simplified the problem into a small set of requirements, which if a proper solution is provided, will help me to solve my problems. Problem 1 is to determine what is the correct pattern to use, since the most obvious solution (Wait) is considered wrong. Problem 2 is the performance issue, which I'm not sure has a solution once one is forced down the async path, but maybe there is so I'm open to suggestions.


    • Edited by BitFlipper Monday, June 23, 2014 2:46 PM
    Monday, June 23, 2014 2:14 PM
  • Clarification: using Wait on the UI thread is considered Doing It Wrong(TM).

    You can block your background thread however is appropriate.

    That said, if you're trying to do real time audio processing you'll want to use C++. You'll have better low level media library support and deterministic memory management. 

    Monday, June 23, 2014 2:54 PM
    Owner
  • Clarification: using Wait on the UI thread is considered Doing It Wrong(TM).

    You can block your background thread however is appropriate.

    That said, if you're trying to do real time audio processing you'll want to use C++. You'll have better low level media library support and deterministic memory management. 

    OK thanks for the clarification re Wait on non-UI threads. I haven't seen this mentioned that it applies to just the UI thread.

    However your comment to use C++ is wrong. I've been using C# to perform low-level, low-latency audio for a long time now. I've never had a problem until I started being subjected to the async pattern. So there is nothing wrong with C#, the problem is with async. And if I switch to C++, I'll still be subjected to async so I'm not sure how that will solve the problem. Basically you are suggesting I switch languages because async caused a new problem, but you don't have a real answer to this new problem.

    Monday, June 23, 2014 3:06 PM
  • My suggestion had nothing to do with async. It had to do with low level media library support and deterministic memory management.

    That said, C++ would let you completely avoid async for audio processing in a background thread (assuming your cache is in app data and doesn't require the file broker for access)

    --Rob

    Monday, June 23, 2014 3:16 PM
    Owner
  • Rob,

    I'm stuck with async because I have no choice, whether using C# or C++. I'm writing a WinRT app, and the only option there is ReadAsync, see here and here. I'm open to suggestions on how to read the file that don't involve async.

    So I'm not sure what you mean with C++ will let me completely avoid async.

    Again rewriting everything in C++ isn't the answer (for performance reasons - that wasn't the question btw). This works perfectly fine in C# in a non-WinRT app where async can actually be avoided.

    Sorry for sounding annoyed about this issue, but this has been extremely frustrating. I really don't see why MS didn't also provide StorageFile.Read in addition to StorageFile.ReadAsync. Some people actually know how to do multithreaded programming and don't need their hand held by being subjected to async. Not everything is about the UI thread.



    • Edited by BitFlipper Monday, June 23, 2014 4:21 PM
    Monday, June 23, 2014 3:48 PM
  • Hopefully this will help you do what you need, actually I am sure it will!

    public class ApplicationCache
    {
        private static ApplicationCache instance;
    
        private object cacheLock;
    
        private Dictionary<string, object> storage;
    
        private ApplicationCache()
        {
            cacheLock = new object();
    
            storage = new Dictionary<string, object>();
        }
    
        public static ApplicationCache Singelton
        {
            get
            {
                if (instance == null)
                {
                    instance = new ApplicationCache();
                }
    
                return instance;
            }
        }
    
        public object GetData(string key)
        {
            object data = null;
    
            lock (cacheLock)
            {
                if (storage.ContainsKey(key))
                {
                    data = storage[key];
                }
            }
    
            return data;
        }
    
        public void AddOrUpdateData(string key, object data)
        {
            lock (cacheLock)
            {
                if (storage.ContainsKey(key))
                {
                    storage[key] = data;
                }
                else
                {
                    storage.Add(key, data);
                }
            }
        }
    
        public void RemoveData(string key, object data)
        {
            lock (cacheLock)
            {
                if (storage.ContainsKey(key))
                {
                    storage.Remove(key);
                }
            }
        }
    }
    
    public class SampleConsumer
    {
        public async Task<object> GetData(string key)
        {
            object data = null;
    
            data = ApplicationCache.Singelton.GetData(key);
    
            if (data == null)
            {
                data = await readData(key);
    
                ApplicationCache.Singelton.AddOrUpdateData(key, data);
            }
    
            return data;
        }
    
        private async Task<object> readData(string key)
        {
            //Just a sample delay
            await Task.Delay(10);
            //Read here from where you need
            return new object();
        }
    }


    -- Vishal Kaushik --

    Please 'Mark as Answer' if my post answers your question and 'Vote as Helpful' if it helps you. Happy Coding!!!

    Tuesday, June 24, 2014 4:50 AM
  • Vishal,

    Thanks for taking the time with your detailed reply! I can see what your sample code does but I can't see how multiple dedicated background threads will access the data in the cache.

    Can you show how each of those dedicated background threads will access the cache in a thread-safe way?

    Below is some sample code to show what I mean:

    public class Test
    {
        public void StartBackgroundProcessing(int threadCount)
        {
            for (var idx = 0; idx < threadCount; idx++)
            {
                Task.Run(() =>
                {
                    DedicatedBackgroundLoop();
                });
            }
        }
    
        private bool exit;
    
        private void DedicatedBackgroundLoop()
        {
            var dataIndex = 0;
    
            while (!exit)
            {
                var data = GetData(dataIndex++);  //???
      
                ProcessData(data);
            }
        }
    
        private void ProcessData(int data)
        {
            // Process the data here
        }
    }

    EDIT: Just to clarify about the dedicated threads... They process audio data in realtime, and their thread priority is boosted which is what one does to achieve low latency glitch-free audio. The thread priority boost is not shown in the sample code, but just note that they cannot be replaced with generic threadpool threads etc.
    • Edited by BitFlipper Tuesday, June 24, 2014 3:40 PM
    Tuesday, June 24, 2014 3:31 PM
  • Please don't implement your own thread-safe collection types.  There are perfectly good ones shipped with the .NET Framework. 

    For instance

    ConcurrentQueue<T>

    and

    BlockingCollection<T>

    BlockingCollection is especially handy for background processes as it will allow background threads to wait on new work.

    Also your tasks should be marked as LongRunning if they run in a loop. EG:

     public class Test
        {
    
            BlockingCollection<WorkItem> workQueue = new BlockingCollection<WorkItem>();
    
            List<Task> tasks = new List<Task>();
    
            public void StartBackgroundProcessing(int threadCount)
            {
                
                for (var idx = 0; idx < threadCount; idx++)
                {
                    var t = new Task(DedicatedBackgroundLoop, TaskCreationOptions.LongRunning);
                    tasks.Add(t);
                    t.Start();
                }
            }
    
            private bool exit;
    
            private void DedicatedBackgroundLoop()
            {
                while (!exit)
                {
                    WorkItem  data;
                    if (!workQueue.TryTake(out data, 500) )
                    {
                        continue;
                    }
                    
                    ProcessData(data);
                }
            }
    
            private void ProcessData(WorkItem data)
            {
                // Process the data here
            }
        }

    David


    David http://blogs.msdn.com/b/dbrowne/


    Tuesday, June 24, 2014 4:14 PM
  • Guys, this has nothing to do with collections, and this discussion is now getting away from the original problem which is still unanswered.

    There is no queue, there is no collection of any sort. It is purely multiple background threads that need to access data whose source data is only available via an async call (StorageFile.readAsync in my case).

    The particular "work items" don't become available due to some collection having data added into it from somewhere else. Instead, each of the background threads request the data when they are ready to do so, not the other way around.

    I guess people are so used to viewing everything from a UI/web view that they don't realize there are other use cases too. And this part of my frustration. It looks like those implementing these APIs also can't imagine other use cases outside of UI or web. Hence no StorageFile.Read.


    • Edited by BitFlipper Tuesday, June 24, 2014 10:09 PM
    Tuesday, June 24, 2014 10:08 PM
  • > the original problem which is still unanswered.

    Wasn't the original problem: How to wait for IO completion on a background thread?

    And the answer is to call Task.Wait().

    Additionally the performance test you posted indicated that the async invocation took 0.16 ms, but you can't expect file IO to complete in under a few ms, so the added overhead of the async invocation should not be material.

    David


    David http://blogs.msdn.com/b/dbrowne/



    Tuesday, June 24, 2014 10:28 PM
  • Task.Wait would solve the async problem. I doubt it will significantly affect the performance.

    I suspect the latency attributed to async is really due to the interprocess communication to the StorageItem file broker. As I alluded to previously, assuming the cache is in the application data this can be eliminated by calling the file API directly: either using C runtime Stream I/O or Win32 CreateFile2/ReadFile/etc.

    Tuesday, June 24, 2014 11:02 PM
    Owner
  • Rob,

    The performance results I've shown is not using any IO, so the latency we see is not due to that. The test is simply measuring the call latency of a simple non-async vs async call. This is what the non-async and async versions of the call look like this:

    protected virtual int DoSomething()
    {
        return 42;
    }

    protected async virtual Task<int> DoSomethingAsync() { await Task.Yield(); return 42; }

    The performance test also shows that await and Task.Wait (and therefore also Task.Result) results in roughly the same amount of latency. Therefore 0.15 ms is the added overhead when calling into async methods.

    So when we consider the following:

    1. Low level audio processing threads usually have their thread priority boosted. This isn't just some made up requirement from my side, this is what MS actually advocates for these types of threads, see here, here and here.
    2. When any of these high priority threads have to use await, Task.Wait or Task.Result, it will result in thread priority inversion. See here and here.

     

    So the problem is a bit more complex and using Task.Wait is not the answer. The correct answer is to keep async/await out of these threads altogether. However seeing the current overzealous trend of async everywhere, I'm concerned that people that don't understand these types of use cases will be making bad decisions. We already see this with StorageFile.ReadAsync where there is no sync version available. Will all new APIs only have async versions from now on?



    • Edited by BitFlipper Wednesday, June 25, 2014 1:17 PM
    Wednesday, June 25, 2014 1:16 PM
  • >When any of these high priority threads have to use await, Task.Wait or Task.Result, it will result in thread priority inversion.

    Probably, but synchronous IO would do the same thing.  The priority inversion will happen when your thread encounters _any_ wait. 

    When you perform a synchronous file read (ReadFile), your thread will go into a wait while the data is read from disk.  The duration of this wait could be under 1ms if the data is in cache or on an SSD, but is likely to be 2ms-10ms if the data is read from an ordinary hard disk.  But in any case synchronous IO typically involves a thread wait, which causes the thread to give up its quantum, which means that it will be as much as ~10ms until the thread can resume.

    If you want to keep your thread scheduled while you wait for IO, you can do that with

    SpinWait.SpinUntil( () => task.IsCompleted );

    or

    SpinWait.SpinUntil( () => asyncOperation.Status == AsyncStatus.Completed );
    

    David


    David http://blogs.msdn.com/b/dbrowne/



    Wednesday, June 25, 2014 1:41 PM
  • David,

    Thanks for your reply. Using SpinWait as you suggested looks interesting, I'll do some tests to get an idea how it can help.

    As far as the disk IO is concerned, I agree with you so using StorageFile.ReadAsync probably isn't the best example I can use to explain my point. My concern is that more and more APIs might get async-only versions in the future (we seem to be headed that way) which can then cause problems for these kinds of scenarios.

    BTW, do you know if I would be possible to implement my own versions of SynchronizationContect and/or TaskScheduler that can then be associated with these higher priority threads? The idea is to make all calls on those threads be synchronous. The descriptions of these classes specifically mention handling different threading scenarios.

    Wednesday, June 25, 2014 1:53 PM
  • David,

    I tested your suggestion to use SpinWait. It results in a reduction of call latency of roughly 50%, which is a nice improvement but still far from ideal, see results below:

    ----------------

    Sync:   00:00:00.0022946 which is 0.000023 ms per op
    Async1: 00:00:14.6293509 which is 0.146294 ms per op     <- await
    Async2: 00:00:07.9690130 which is 0.079690 ms per op     <- SpinWait

    Async1 is 6375.6 times slower
    Async2 is 3472.9 times slower

    ----------------

    I also checked CPU usage as I expected SpinWait to consume more CPU, but was surprised to see no difference between Async1 and Async2 CPU usage (around 5% for both).


    • Edited by BitFlipper Wednesday, June 25, 2014 2:20 PM
    Wednesday, June 25, 2014 2:19 PM
  • Like David said, your test demonstrates that yielding in a loop is slower than a noop. That isn't related to the async/await system. I can demonstrate the same thing by yielding in a loop without using the async system.

    There is some overhead for an async call to handle the completion state machine, but in most cases (and in particular with StorageFile.ReadAsync) this will be trivial compared to the cost of the actual work the async function does.

    You want to avoid any delays such as yielding, crossing the ABI, garbage collection, etc. inside tight loops.

    Ignoring the hypothetical discussion of the evils of async, for your specific case using direct file API rather than brokered API will significantly help performance by removing the IPC.

    --Rob

    Wednesday, June 25, 2014 9:53 PM
    Owner