3
\$\begingroup\$

I am writing a piece of software that aims at streaming the modified lines of an external file located at given uri (and retrieved as a QueryString parameter).

I am trying to minimize the impact of big files so that I can maximize the use of asynchronous calls (i.e. async / await). I am using AsyncEnumerable (https://github.com/Dasync/AsyncEnumerable) to have at the same time the feature of enumerable and and async / await, in order to free up threads and memory (I am not going to keep the whole file in memory before sending the modified version back).

I created an extension method that can fetch the lines of a remote file.

public static class StringExtensions
{
    public static AsyncEnumerable<string> ReadLinesAsyncViaHttpClient(this string uri)
    {
        return new AsyncEnumerable<string>(async yield =>
        {
            using (var httpClient = new HttpClient())
            {
                using (var responseStream = await httpClient.GetStreamAsync(uri))
                {
                    using (var streamReader = new StreamReader(responseStream))
                    {
                        while(true)
                        {
                            var line = await streamReader.ReadLineAsync();

                            if (line != null)
                            {
                                await yield.ReturnAsync(line);
                            }
                            else
                            {
                                return;
                            }
                        } 
                    }
                }
            }
        });
    }
}

And used that extension method in a Controller action:

using System;
using System.Collections.Async;
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;

namespace WebApplicationTest.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    public class LinesController : ControllerBase
    {
        private static readonly Random Random = new Random();

        // GET api/lines
        // POST api/lines
        [HttpGet]
        [HttpPost]
        public async Task<IActionResult> GetAsync([FromQuery] string fileUri)
        {         
            using (var streamWriter = new StreamWriter(HttpContext.Response.Body))
            {
                await fileUri.ReadLinesAsyncViaHttpClient().ForEachAsync(async line =>
                {
                    // Some dumb process on each (maybe big line)
                    line += Random.Next(0, 100 + 1);
                    await streamWriter.WriteLineAsync(line);
                });
            }

            return Ok();
        }
    }
}

I used the HttpContext.Response.Body stream instead of the PushStreamContent cause according to this answer: https://stackoverflow.com/a/48115434/4636721 (in .NET Core we don't need to use the PushStreamContent if no need of HttpMessage)

It seems to work I mean, I get a payload with the lines of the initial file passed in the query string and with some random numbers at the end of each line. It seems there is no increase in memory but I would like to have another pair of eyes to check what I have done.

[EDIT 2] Turns out there is already an issue on the ASP.NET MVC Github about that: https://github.com/aspnet/Mvc/issues/6224

\$\endgroup\$
  • 4
    \$\begingroup\$ yield - this is a really, really, really! terrible variable name. I had to look ten times at your code to actually understand that it's not an enumerator :-o \$\endgroup\$ – t3chb0t Nov 5 '18 at 19:24
  • \$\begingroup\$ @t3chb0t: this is due AsyncEnumerable github.com/Dasync/AsyncEnumerable it's a workaround for the actual keyword which cannot be used pre C# 8 (if Async Streams are accepted someday): github.com/Dasync/… \$\endgroup\$ – Ehouarn Perret Nov 5 '18 at 19:33
  • \$\begingroup\$ Thanks for the links. This is an interesting idea but I'm still sceptical about their word choice. It somehow doesn't fit well. \$\endgroup\$ – t3chb0t Nov 5 '18 at 19:57
  • \$\begingroup\$ @t3chb0t hm may worth replace it with "yielder". \$\endgroup\$ – Ehouarn Perret Nov 5 '18 at 20:01
  • \$\begingroup\$ @t3chb0t, that's the point of the library - to make asynchronous enumeration as easy as the built-in synchronous counterpart. You don't need to spend a lot of time learning how use it due to high similarity in syntax. The Author. \$\endgroup\$ – Serge Semenov Nov 27 '18 at 18:54
1
\$\begingroup\$

As the author of the AsyncEnumerable library, I confirm the correctness of the code.

P.S. we've been using this library for 3 years at the heart of a large distributed software at my current workplace.

\$\endgroup\$
  • \$\begingroup\$ there are two things I am concerned with: 1] is whether the controller is going to buffer the data while they are being streamed to the Response.Body and increase memory use in the ASP.NET Core WebAPI application 2] how to properly handle errors, cause once you write to the stream you cannot modify the http status code in the response... I read in some places that you can add a middleware catching specific exceptions but it seems a bit over-convoluted \$\endgroup\$ – Ehouarn Perret Nov 28 '18 at 0:35
  • 1
    \$\begingroup\$ @EhouarnPerret, the GetStreamAsync should guarantee the streaming and not loading the entire body, as 'under the hood' it uses the HttpCompletionOption.ResponseHeadersRead option. Regarding error handling, yes, it can be tricky. Essentially it's a typical problem of cascading failures of synchronous calls. On error, you can potentially re-issue the request and skip first X lines that were already returned. There are other options, but that's a bigger software architecture problem. \$\endgroup\$ – Serge Semenov Nov 28 '18 at 4:30
  • \$\begingroup\$ thank you man! Really appreciated, I think I will keep using my middleware trick for error handling and update my post accordingly. \$\endgroup\$ – Ehouarn Perret Nov 28 '18 at 21:51
2
\$\begingroup\$

A small improvement you can do to avoid the pretty deep nesting is, stacking all usings without extra brackets.

public static AsyncEnumerable<string> ReadLinesAsyncViaHttpClient(this string uri)
{
    return new AsyncEnumerable<string>(async yield =>
    { 
        using (var httpClient = new HttpClient())
        using (var responseStream = await httpClient.GetStreamAsync(uri))
        using (var streamReader = new StreamReader(responseStream))
        {
            while(true)
            {
                 var line = await streamReader.ReadLineAsync();
                 if (line != null)
                 {
                     await yield.ReturnAsync(line); 
                 }
                 else
                 {
                     return;
                 } 
             }
        }
    });
}

Apart from that I don't know if this the kind of thing that should be a extension method. This should rather be a class.

(I'm currently do not have my hands on a pc, so I cannot comment on the technical side of things >:( )

\$\endgroup\$
  • \$\begingroup\$ Yeah I am aware of that, I wanted to keep the braces to see clearly what is nested. Curious about your thoughts on the technical side. \$\endgroup\$ – Ehouarn Perret Nov 6 '18 at 19:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.