1

I have a piece of code that execute HTTP requests (inside a loop) from some currency API and do some work on the data which received.

Because it was slow, I changed it to asynchronously function using: Task.Run()

However, when I measure calculation time (using Stopwatch..) of the function with the async method, it takes more time (or even) than the sync one.

How can it possible? Am I do it wrong?

[HttpPost,Route("GetCurrenciesData")]
public IHttpActionResult GetCurrenciesData(InputCurrenciesData cls)  
{
    try
    {
        var watch = Stopwatch.StartNew();

        var data2 = GetBL.mySyncMethod(cls.currenciesList);

        watch.Stop();
        string first = watch.Elapsed.ToString();  // --> this faster
        watch = Stopwatch.StartNew();
        var data = GetBL.myAsyncMethod(cls.currenciesList);
        string second = watch.Elapsed.ToString();  // --> this slower
        return Ok(data);
    }
    catch (Exception ex)
    {
        getGeneric.WriteError(ex.Message, ex.StackTrace);
        return BadRequest();
    }
}

Example code of the sync function:

public GenericClass mySyncMethod(string[] currencies)
{
        try
        { 
            foreach (string currency in currencies)
            {

                getDataFromApi(currency);
            }


            return getDataChart;

        }
        catch (Exception ex)
        {
            WriteError(ex.Message, ex.StackTrace);
            return null;
        }
}

Example of the async :

public GenericClass myAsyncMethod(string[] currencies)
    {
        try
        {
            List<Task> TaskList = new List<Task>();

            foreach (string currency in currenies)
            {

                var myTask = new Task(() =>
                {
                    getDataFromApi(currency);
                });
                myTask.Start();
                TaskList.Add(myTask);

            }

            Task.WaitAll(TaskList.ToArray());

            return getDataChart;

        }
        catch (Exception ex)
        {
            WriteError(ex.Message, ex.StackTrace);
            return null;
        }
    }

The code of the function getDataFromApi:

private void getDataFromApi(string currency)
    {

        string currencyCode = getCurrenciesDictionary[currency];

//Get api's URL from Web.config and concatenation the wanted currency code
        string URL = string.Format(GetConfig("Api"), currencyCode);  

        /*HTTP request to retrieve data from Api*/
        using (HttpClientHandler handler = new HttpClientHandler())
        {
            handler.UseDefaultCredentials = true;
            using (HttpClient httpClient = new HttpClient(handler))
            {
                var ResultAsync = httpClient.GetAsync(URL).Result;
                if (ResultAsync.IsSuccessStatusCode)
                {
                    var content = ResultAsync.Content;

                    string Data = content.ReadAsStringAsync().Result;

                    try
                    {
                        var ret = JsonConvert.DeserializeObject<RootObject>(Data);


                        /*add new class to currencyRate list - class which represent the currency and its rates*/
                        getDataChart.CurrencyRate.Add(new CurrencyRate
                        {
                            Currency = currency,
                            Rates = ret.dataset.data.AsEnumerable().
                                    Select(date => Double.Parse(date[1].ToString())).ToList()
                        });


                    }
                 catch (Exception ex)
                 {
                    WriteError(ex.Message + " currency: " + currency + "/n/ URL: " + URL, ex.StackTrace);
                }

             }
      }
}
O. H
  • 77
  • 8
  • 2
    Yes, it is possible. There is a (small) overhead cost of creating and handling tasks. If you create a large amount of tasks, each of which only makes minor gains, it's possible that the overhead costs outweigh the benefits and therefore everything runs slower. I leave finding the issue in your code up to others since I'm not that great at async myself :) – Flater May 17 '18 at 06:55
  • 1
    Yes. Glater is right. In your code, your are using var ResultAsync = httpClient.GetAsync(URL).Result; Don't do it and await on the GetAsync. If you need it synchronous in the futures (not here). Use GetAwaiter().GetResult() https://stackoverflow.com/questions/17284517/is-task-result-the-same-as-getawaiter-getresult – Marco May 17 '18 at 06:58
  • An async method will always be slightly slower than its synchronous counterpart. But what you lose in latency, you gain it on throughput by reducing the amount of resources used. Latency and throughput are two opposites: optimizing for one often means worsening the other – Kevin Gosse May 17 '18 at 06:58
  • The other problem is Task.WaitAll(TaskList.ToArray()); Better use await Task.WhenAll ... – Marco May 17 '18 at 07:00
  • @Marco his name is Flater, not Glater – Cid May 17 '18 at 07:02
  • @Cid Flater is name? – JohnyL May 17 '18 at 07:03
  • 1
    Whether async has overhead or not, but I can see code does not follow pure async apporach. It invokes the sync code inside async code which is not correct. Basically, the whole call hierarchy should follow the async await pattern to have better throughput. – user1672994 May 17 '18 at 07:04
  • You have an I/O problem, Task.Run() won't help you there. You are using 'async' completely wrong. – bommelding May 17 '18 at 07:26
  • OK so after I read all these comments and more about async/await and change `.Result` to await, and all the stuff, the actual time didn't change and still the same (+-) like the sequentially. I it to `Parallel.ForEach(currencies, currency => { sync code });' and not it has a better performance than the sequentially one (30% faster). someone have a better idea or in this case Parallel.ForEach is the way? – O. H May 17 '18 at 12:13

1 Answers1

3

try and read some of this guy:

he's telling in a few words how it works. The main point is that you get some overhead using async/await, however, you're freeing resources. So any task that will run, will run on the synchronized context. but you can handle much more request.

Besides the fact that you are not really using async functionality with the .result kills actually the async part.

In a short line, doing it async doesn't necessarily speed it up. If you have more CPU-bound tasks, that can run simultaneously you will major performance, however, in your case, you will not performance, but resources like Kevin tells in his comments.

hope it helps!

Roelant M
  • 1,581
  • 1
  • 13
  • 28