2
\$\begingroup\$

I have a queue of jobs in an ASP.NET CORE Application. Each of the files are in a restricted area and I have to Login first, thats why I use a Cookie Container for my download request.

The Job is logging into a platform and then starts to download the file, given an URL and a Cookie Container from the Login.

The download method is looking like this and has a Func which reports the progress in 5 % steps:

public async Task DownloadFile(string url, string destination, Func<int, Task> reportStatus)
{
        await reportStatus(0);

        var request = WebRequest.CreateHttp(url);
        request.CookieContainer = m_cookieContainer;
        var response = (HttpWebResponse)await request.GetResponseAsync();
        var requestStream = response.GetResponseStream();

        using (var reader = new BinaryReader(requestStream))
        {
            using (var fileStream = File.OpenWrite(destination))
            {
                const int bufferSize = 2048;
                byte[] bytes;
                short oldPercentage = 4;
                long bytesRead = 0;
                do
                {
                    bytes = reader.ReadBytes(bufferSize);
                    await fileStream.WriteAsync(bytes, 0, bytes.Length);

                    bytesRead += bytes.Length;

                    var percentage = (short)(bytesRead * 100 / response.ContentLength);
                    if (oldPercentage < percentage)
                    {
                        await reportStatus(percentage);
                        oldPercentage = (short)(oldPercentage + 5);
                    }
                } while (bytes.Length != 0);
            }
        }

        await reportStatus(100);
}

Small problems I am currently facing:

  • Download speed feels quite slow in comparison to my internet connection.
  • I was wondering if there are better ways downloading a file from a restricted area using .NET Core functionality.
\$\endgroup\$
5
  • \$\begingroup\$ i think parameter jobId is useless here \$\endgroup\$ May 19 '17 at 8:16
  • \$\begingroup\$ maybe speed will increase if you write all bytes to file once in the end \$\endgroup\$ May 19 '17 at 8:21
  • \$\begingroup\$ For simplicity I have made the source code a bit smaller, thats why Job id isn't used. I have removed it from the code now. \$\endgroup\$
    – Jannik
    May 19 '17 at 8:42
  • \$\begingroup\$ @Disappointed memory usage will also then increase \$\endgroup\$
    – cat
    May 19 '17 at 10:15
  • 1
    \$\begingroup\$ I have rolled back your edit as it would invalidate existing answers, in accordance with our policy.See What you may and may not do after receiving answers \$\endgroup\$ May 19 '17 at 13:03
7
\$\begingroup\$

You mentioned that the download is slower than you'd expect, I'll focus on just that for a quick review.

Look at your code here:

var percentage = (short)(bytesRead * 100 / response.ContentLength);
if (oldPercentage < percentage)
{
    await reportStatus(percentage);
    oldPercentage = (short)(oldPercentage + 5);
}

Can you see why that might be slowing down your download?

await reportStatus(percentage);

You're waiting for the task reporting the status to finish before reading the rest of the stream! What if reportStatus were logging the progress to a database or doing something else really time consuming?

I think you should just execute the callback and not care when/how it finishes:

reportStatus(percentage); // Don't wait

Oh and oldPercentage is really confusing when you scan the code... I'd rename it to something like reportingThreshold.

if (percentage > reportingThreshold)
{
     reportStatus(percentage);
     reportingThreshold = (short)(reportingThreshold+ 5);
}
\$\endgroup\$
2
  • \$\begingroup\$ Yeah, I saw that right after I posted this question. It actually is a bottleneck, because I do write to the Job database everytime I report the progress. However it is only updated every 5 %, so it can't be the main problem here when it comes to big files (e.g. 100 MB downloading with a 50mbit connection). Anyway, thanks I will try to implement your change :) I will get into some threading issues though. Because who will guarantee that 5 % DB update is committed before a 10 % DB update :) \$\endgroup\$
    – Jannik
    May 19 '17 at 8:51
  • 1
    \$\begingroup\$ @Jannik - Add your updates to a queue in reportProgress and process the queue separately to maintain the order. I can assure you that the latency on a DB call 20 times during a download will have a noticeable effect on speed. \$\endgroup\$
    – RobH
    May 19 '17 at 9:13
5
\$\begingroup\$

As a matter of fact you should use the IProgress<T> interface for reporting progress that

Defines a provider for progress updates.

With it you wouldn't be tempted to await it because the Report method is void.

Example:

public async Task DownloadFile(string url, string destination, IProgress<int> progress)
{
   ..
   progress.Report(percentage);
   ..
}

See also Async in 4.5: Enabling Progress and Cancellation in Async APIs

\$\endgroup\$
0

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.