Skip to content

Conversation

@PatrickJnr
Copy link
Contributor

  • Converted implementation to use async/await pattern.
  • Implemented parallel downloads using Parallel.ForEachAsync.
  • Optimised verification to check file size before calculating the hash.
  • Added on-the-fly hashing during download to reduce disk I/O.
  • Added file pre-allocation and cancellation token support.

@PatrickJnr PatrickJnr force-pushed the optimize-download-artifacts branch from 78efba5 to 127b884 Compare December 5, 2025 09:08
@lolleko
Copy link
Contributor

lolleko commented Dec 5, 2025

nice some god optimizations, I definitely understand these:

  • Optimised verification to check file size before calculating the hash.
  • Added on-the-fly hashing during download to reduce disk I/O.
  • Added file pre-allocation and cancellation token support.

However, I am not good with the async/await and cant judge the impact, how does slapping it everywhere improve perf? Before it ran in parallel is well, so I wonder if this will make any difference, have you profiled? I think the code looks more complex with async/await.

@PatrickJnr
Copy link
Contributor Author

PatrickJnr commented Dec 5, 2025

The​‍​‌‍​‍‌​‍​‌‍​‍‌ main advantage here is not necessarily a faster processor, but rather how the system efficiently handles input/output operations and thread management due to the new streaming pipeline. It doesn't work like this:

Reduced Disk Activity: We do not save and then read a file to hash it, but the hash calculation is done on the fly while the file is being downloaded.
Improved Thread Management: The use of Parallel.ForEachAsync allows threads to be released while waiting for network data, minimising the need for task switching, which is almost completely eliminated.
The Big Picture: Because we are streaming the writing and hashing operations, we are able to greatly reduce the amount of data that is written to the disk, which in most cases is the major factor of the slowdown when dealing with this kind of ​‍​‌‍​‍‌​‍​‌‍​‍‌files.

If the code is too complex, feel free to close the PR, thank you for reviewing the code :)

@lolleko
Copy link
Contributor

lolleko commented Dec 5, 2025

PR is not to complex. I just would love to see some actual numbers that this is an improvement.
Since this is not even compiling I am also wondering if you tested this at all.

@PatrickJnr
Copy link
Contributor Author

That's strange, it compiled fine locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants