Usage of TPL vs Parallel.ForEach() on file processing

Question

i've been working on an implementation of a my own from scratch set of library Astron and I wanted to get my TPL usage reviewed because i'm not confident with this technology :/

My app is currently unpacking .d2p files, which is a custom file format from a french game called Dofus, it's simply an archive of others inflated archives of another custom file format .dlm. Here is the current output of my app :

application output The progress bars came from here.

So everything seems to works as expected, files are processes concurrently, 308mo of .dlm files are parsed deflated in 10sec, that is exactly what i wants it to do but i may have misused the TPL. The full project can be found at this address, but the portion of code I want to get reviewed is from src/Astron.Unpacker/Managers/D2PManager.cs :

public class D2PManager : BaseFileManager
{
 private readonly ILogger _logger;
 private readonly string _dlmFilesFolder;
 public D2PManager(IContainer container) : base(container)
 {
 _logger = ServiceLocator.Logger;
 _dlmFilesFolder = container.GetInstance<Settings>().DlmOutputFolder;
 }
 public async Task UnpackAll(string[] filesPath)
 {
 _logger.Log<D2PManager>(LogLevel.Info, $"Attempting to unpack {filesPath.Length} d2p files...");
 var tasks = new List<Task>(filesPath.Length);
 tasks.AddRange(filesPath.Select(d2PFilePath => UnpackD2PFile(d2PFilePath)));
 await Task.WhenAll(tasks).ConfigureAwait(false);
 }
 public async Task UnpackD2PFile(string path)
 {
 var d2PFile = new FileAccessor(path);
 var metaUnpacker = new D2PFileMetadataUnpacker(_binaryFactory, _serDes);
 metaUnpacker.Unpack(d2PFile);
 var archiveUnpacker = new DlmArchivesUnpacker(_binaryFactory, _serDes, metaUnpacker.Value);
 archiveUnpacker.Unpack(d2PFile);
 var progressCount = 1;
 var progressBar = new ProgressBar(PbStyle.SingleLine, archiveUnpacker.Values.Count);
 progressBar.Refresh(0, Path.GetFileName(d2PFile.FullPath));
 await Task.Delay(10); // doesn't print all either way
 foreach (var archive in archiveUnpacker.Values)
 {
 var filePath = (_dlmFilesFolder + archive.RelativePath).Replace('/', '\\');
 var fileDirectory = Path.GetDirectoryName(filePath);
 using var decompressedData = new MemoryStream();
 using var deflatedStream = new DeflateStream(new MemoryStream(archive.CompressedData), 
 CompressionMode.Decompress);
 await deflatedStream.CopyToAsync(decompressedData);
 if (!Directory.Exists(fileDirectory)) Directory.CreateDirectory(fileDirectory);
 File.WriteAllBytes(filePath, decompressedData.GetBuffer());
 progressBar.Refresh(progressCount, filePath);
 progressCount++;
 }
 }
}

Here are my questions :

• If I don't add the Task.Delay() right after the processbar initialization, the files seems to be processed synchronously (the progress bar show up when the last completed), why does it happen ?
• Is it right to use .ConfigureAwait(false) on Task.WhenAll() ?
• Am I starting every tasks the right way ? Shouldn't I use Task.Run() instead with Task.WaitAll() ?

• dfhwze: At first glance, I would say remove the ConfigureAwait(false) and await Task.Delay(10) and see what happens...
• t3chb0t: I would replace your current Task.WhenAll with Parallel.ForEach(filesPath, ...) and let the framework create and handle the concurrency or alternatively filesPath.AsParallel()... btw, you are not disposing any streams.
• Nameless: @dfhwze without the task.delay files are processed synchronously according to this output: i.imgur.com/24O9ZzX.gif as I said on the question, why should I remove the configure await ?
• Nameless: @t3chb0t yeah i was doing this before but Parallel foreach doesn't ensure that everything will be processed concurrently, thank you for the not disposed streams, i didn't noticed that, thanks to C#8 i just have to add using before the var definition :)
• t3chb0t: async Task requires an await, if there is nothing else you can await then I think you should return Task.CompltetedTask. and use Task.Factory.StartNew (() => ) to call this method... but I looked through your repository and you could add some awaitable methods to make it naturally awaitable but this would require changing a lot of other APIs.

Answer 1

As seen with @t3chb0t and @dfhwze I have been misusing the TPL. I also don't see any point on .ConfigureAwait(false) a Task.WhenAll() call, also with the async all-the-way rule, using an awaitable task for this case would have me to convert many of my APIs to async one.

So they provided me these solutions :

• Modifiy my task creation logic this way : tasks.AddRange(filesPath.Select(async d2PFilePath => await Task.Run(() => UnpackD2PFile(d2PFilePath)))); and removing the Task.Delay(10); on my UnpackD2PFile(string path) task
• Use the Parallel.Foreach() loop to let the framework create and handle the concurrency and also allow me to keep a synchronous API

Both of these solutions completed the work in the same time, and provided this output :

new application output

As you can see now the behavior is quite different than before, not every task start at the same moment, also now there is a delay before every progress bars shown up. But we gained almost 2sec on the total execution time :)

I highly recommend to use the Parallel approach as it is the easiest one to implement.