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 :
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)
onTask.WhenAll()
? - Am I starting every tasks the right way ? Shouldn't I use
Task.Run()
instead withTask.WaitAll()
?
Task.WhenAll
withParallel.ForEach(filesPath, ...)
and let the framework create and handle the concurrency or alternativelyfilesPath.AsParallel()...
btw, you are not disposing any streams. \$\endgroup\$ – t3chb0t May 19 '19 at 8:09async Task
requires anawait
, if there is nothing else you canawait
then I think you shouldreturn Task.CompltetedTask.
and useTask.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. \$\endgroup\$ – t3chb0t May 19 '19 at 8:26