3

We got an existing library where some of the methods needs to be converted to async methods.

However I'm not sure how to do it with the following method (errorhandling has been removed). The purpose of the method is to zip a file and save it to disk. (Note that the zip class doesn't expose any async methods.)

public static bool ZipAndSaveFile(string fileToPack, string archiveName, string outputDirectory)
{
    var archiveNameAndPath = Path.Combine(outputDirectory, archiveName);
    using (var zip = new ZipFile())
    {
        zip.CompressionLevel = Ionic.Zlib.CompressionLevel.BestCompression;
        zip.Comment = $"This archive was created at {System.DateTime.UtcNow.ToString("G")} (UTC)";
        zip.AddFile(fileToPack);
        zip.Save(archiveNameAndPath);
    }
    return true;
}

An implementation could look like this:

public static async Task<bool> ZipAndSaveFileAsync(string fileToPack, string archiveName, string outputDirectory)
{
    var archiveNameAndPath = Path.Combine(outputDirectory, archiveName);
    await Task.Run(() => 
    {
        using (var zip = new ZipFile())
        {
            zip.CompressionLevel = Ionic.Zlib.CompressionLevel.BestCompression;
            zip.Comment = $"This archive was created at {System.DateTime.UtcNow.ToString("G")} (UTC)";
            zip.AddFile(fileToPack);
            zip.Save(archiveNameAndPath);
        }
    });
    return true;
}

Which just seems wrong. The client could just call the sync method using Task.Run

Please, anyone got any hints on how to transform it into a async method ?

9
  • 3
    Zipping files is a CPU bound operation, there's nothing naturally async about it. Why are you required to transform these methods? The only thing that can be async is reading from disk. Commented Aug 6, 2015 at 9:47
  • There is nothing you can do about it. Zipping is not inherently asynchronous. If client code needs to await it, wrap it in Task.Run Commented Aug 6, 2015 at 9:51
  • 1
    @YuvalItzchakov I think he's trying to save it asynchronously. Commented Aug 6, 2015 at 9:52
  • 1
    @Aron The operation (the actual zipping of the content) is a CPU bound operation. I wasn't talking about disk IO for saving and reading. Commented Aug 6, 2015 at 9:58
  • 2
    @Aron it's actually 3 logical stages: 1. Read the source file (IO) 2. Compress the source (CPU) 3. Write the output (IO) 1 & 3 would benefit from being made asynchronous, 2 could do depending on the size of the files being zipped to offload the work to a background thread to keep the application responsive (in a windows app for example) Commented Aug 6, 2015 at 10:05

5 Answers 5

1

Which just seems wrong. The client could just call the sync method using Task.Run

Spot on. By wrapping synchronous code in Task.Run() the library is now using the client's threadpool resources without it being readily apparent. Imagine what could happen to the client's threadpool if all libraries took this approach? Long story short, just expose the synchronous method and let the client decide if it wants to wrap it in Task.Run().

Having said that, if the ZipFile object had async functionality (e.g. had a SaveAsync() method) then you could make the outer method async as well. Here's an example of how that would look:

public static async Task<bool> ZipAndSaveFileAsync(string fileToPack, string archiveName, string outputDirectory)
{
    var archiveNameAndPath = Path.Combine(outputDirectory, archiveName);
    using (var zip = new ZipFile())
    {
        // do stuff
        await zip.SaveAsync(archiveNameAndPath);
    }   
    return true;
}
Sign up to request clarification or add additional context in comments.

Comments

1

As a temporarily solution, I would introduce an extension method:

public static class ZipFileExtensions
{
    public static Task SaveAsync(this ZipFile zipFile, string filePath)
    {
        zipFile.Save(filePath);
        return Task.FromResult(true);
    }
}

Then the usage would be:

public static async Task<bool> ZipAndSaveFileAsync(string fileToPack, string  archiveName, string outputDirectory)
{
    var archiveNameAndPath = Path.Combine(outputDirectory, archiveName);
    using (var zip = new ZipFile())
    {
        ...
        await zip.SaveAsync(archiveNameAndPath).ConfugureAwait(false);
    }
    return true;
 }
  1. Implementing synchronous tasks does not violate anything (talking about Task.FromResult)
  2. Submit a request to https://github.com/jstedfast/Ionic.Zlib asking for an async support in the library due to IO operations
  3. Hope that's done eventually, and then you can upgrade the Ionic.Zlib in your app, delete the ZipFileExtensions, and continue using async version of the Save method (this time built into the library).
  4. Alternatively, you can clone the repo from GitHub, and add SaveAsync by yourself, the submit a pull request back.
  5. It's just not possible to 'convert' a sync method to an async if a library does not support it.

From performance standpoint, this might not be the best solution, but from management point of view, you can decouple stories "Convert everything to async" and "Improve app performance by having Ionic.Zlib async", what makes your backlog more granular.

Comments

-1
 public static Task<bool> ZipAndSaveFileAsync(string fileToPack, string archiveName, string outputDirectory)
    {

        return Task.Run(() => 
        {
           var archiveNameAndPath = Path.Combine(outputDirectory, archiveName);
            using (var zip = new ZipFile())
            {
                zip.CompressionLevel = Ionic.Zlib.CompressionLevel.BestCompression;
                zip.Comment = $"This archive was created at {System.DateTime.UtcNow.ToString("G")} (UTC)";
                zip.AddFile(fileToPack);
                zip.Save(archiveNameAndPath);
            }
            return true;
        });

    }

Then use like this

public async Task MyMethod() 
{
   bool b = await ZipAndSaveFileAsync();
}

2 Comments

This isn't inherently asynchronous, you're just faking it by using Task.Run.
NEVER use this kind of method.
-1

Some of the answers suggest that zipping a file is not a process that you should do asynchronously. I don't agree with this.

I can imagine that zipping files is a process that might take some time. During this time you want to keep your UI responsive or you want to zip several files simultaneously, or you want to upload a zipped file while zipping the next one/

The code you show is the proper way to make your function asynchronous. You question whether it is useful to create such a small method. Why not let the users call Task.Run instead of call your async function?

The reason for this is called information hiding. By creating the async function you're hiding how you zip asynchronously, thus relieving others from knowing how to do this.

Besides, information hiding gives you the freedom to change the internals of the procedure as long as you don't change the pre- and postcondition.

One of the answers said that your function still is not asynchronous. That is not true. Callers of your function may call your async function without awaiting for it. While the task is zipping, the caller may do other things. As soon as it needs the boolean result of the task if can await for the task.

Example of usage:

private async Task DoSomethingSimultaneously()
{
    var taskZipFileA = ZipAndSaveFileAsync(fileA, ...)
    // while this task is zipping do other things,
    // for instance start zipping file B:
    var taskZipFileB = ZipAndSaveFileAsync(fileB, ...)
    // while both tasks are zipping do other things
    // after a while you want to wait until both files are finished:
    await Task.WhenAll(new Task[] {taskZipFileA, taskZipFileB});
    // after the await, the results are known:
    if (taskZipFileA.Result)
    {
        // process the boolean result of taskZipFile A
    }

Note the difference between Task.WaitAll and Task.WhenAll In async - await you use Task.WhenAll. The return is a Task, so you can

await Task.WhenAll (...)

For proper async-await, all functions that call any async function need to be async themselves and return a Task (instead of void) or Task<TResult> instead of TResult. There is one exception: the event handler may return void.

private async void OnButton1_clicked(object sender, ...)
{
    bool zipResult = await SaveAndZipFileAsync(...);
    ProcessZipResult(zipResult);
}

Using this method your UI keeps responsive. You don't have to call Task.Run

If you have a non-async function and want to start zipping while doing something else, your non-async function has to call Task.Run. As the function is not async it can't use await. When it needs the result of task.Run it needs to use Task.Wait, or Task.WaitAll

private void NonAsyncZipper()
{
    var taskZipFileA = Task.Run ( () => ZipAndSaveFileAsync(...);
    // while zipping do other things
    // after a while when the result is needed:
    taskZipFileA.Wait();
    ProcesZipResult(taskZipFileA.Result);
 }

3 Comments

The author is right, except he doesn't take into account that your async function could work differently than your sync function, For example, the async function could do two things simultaneously, while the caller does something else. You could load the caller with the burden of knowing which things can be done simultaneously and which not, but that would mean that all your million users would have to program the same code and you couldn;t change the internal behaviour anymore.
"The author", Stephen Toub, is a "program manager lead on the Parallel Computing Platform team at Microsoft" :) . You do have a point there, but TMO the async-await pattern was not meant for this kind usage.
-2

If it's possible to get the binary data from your Zip library after the compression, then instead of using this library to save the file, use .NET IO libraries to save it.

EDIT:

There is no point in using async for CPU bound operations (such as compression). In your case, the only benefit you can get from async is when you save the file to the disk. I thought that's what you were asking for.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.