Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add faster file download approach #2672

Closed

Conversation

AlexDickerson
Copy link
Contributor

@AlexDickerson AlexDickerson commented Dec 28, 2024

Issue: When downloading larger modlists Wabbajack can experience periods of low download speed and elevated memory usage. This can significantly slow down the download process.

Cause: The proximate cause of this issue is the usage of the Downloader nuget package for the downloading of files. This package struggles when downloading large quantities of very small files (which many modlists contain thousands of) and it can also lead to elevated memory usage when downloading multiple larger files. The package author indicates that the package is not necessarily suited to downloading large amounts of small files: bezzad/Downloader#158

Solution: Leverage the more modern and supported HttpClient/HttpClientFactory classes to execute these downloads. These are much more performant and are still supported, unlike the .Net Framework classes that are leveraged in the Downloader package.

This adds an additional download client in addition to the current ReusableDownloader. This new client uses the HttpClient to download and stream directly to the disk. Consequently, it is easily able to saturate a 1gb/s connection and because the contents of the files are not persisted memory usage is not a meaningful factor.

This new client does not have the various recovery/resume features of the current approach and as such may be less desirable for those with slower or less reliable connections.

The usage of this path is controlled by a value that can be set in the settings menu that defines the minimum file size at which the ResumableDownloader will be used. It defaults to 0, so anyone who wishes to leverage this new approach will need to increase that setting to a value they feel is appropriate.

The only progress tracking provided with the new approach is when a download starts and finishes, so no progress bars are displayed in the UI but the downloaded files count is updated as expected. Adding static progress bars for downloads should be straightforward. Getting live progress updates would require some non-trivial work. For most downloads this is negligible as they complete so fast but it can be slightly annoying for larger downloads as their status is unknown.

This PR has the ancillary effect of reducing the frequency of UI freezing, though I do not know if that is because of the elimination of progress reporting or if it is related to the downloads themselves.

This PR also creates a new project, Wabbajack.Downloaders.Services, and moves several classes that were previously in Wabbajack.Networking.Http, as to better align these classes to their purpose.

Performance Comparison

Below shows the CPU (red), Memory (blue), and Bytes Received (blue) during the download stage of an installation of LoreRim 3.0 using the approach added here. Memory usage by Wabbajack specifically reached around 5gb at its highest but stabilized around 3gb. Bytes received maintained at ~980Mb/s, decreasing at the end as the number of download streams began to decrease.

image

For comparison, the below shows the same modlist being downloaded in 3.7.5.1 using the default settings. Download speeds fall up and down, reaching a few hundred mb/s periodically but sustained speeds are quite low. Memory usage reached as high as 17gb and was fluctuating around 10-12gb on average. It would need to run for several hours to complete the full download.

image

Below shows the added UI element to control the size at which the download client switches to ResumableDownloader

image

I adjusted the way the Performance settings were being injected in order to make the DI a little cleaner and to deduplicate some code that was being used in two places.

I cleaned up any unnecessary usings and whitespace irregularities in the files I edited as well.

AlexDickerson and others added 27 commits December 27, 2024 16:43
…iles under an arbitrary size

NonReusableDownloader leverages the Downloader library for executing downloads. This presents some performance overhead when downloading large number files, as is often done when downloading larger modlists.

Moved these Downloader classes to the Downloader folder in solution to better align with solution structure.

Added service extension for DI

Cleaned up impacted files

Removed unusued method from ResumableDownloader
Cleaned up code

Removed retry login in NonResumableDownload as DownloadService already retries
Deduplicated two more DI method across OS Integrated and Launcher
…iles under an arbitrary size

NonReusableDownloader leverages the Downloader library for executing downloads. This presents some performance overhead when downloading large number files, as is often done when downloading larger modlists.

Moved these Downloader classes to the Downloader folder in solution to better align with solution structure.

Added service extension for DI

Cleaned up impacted files

Removed unusued method from ResumableDownloader
Cleaned up code

Removed retry login in NonResumableDownload as DownloadService already retries
Deduplicated two more DI method across OS Integrated and Launcher
@AlexDickerson AlexDickerson marked this pull request as ready for review December 28, 2024 22:33
@AlexDickerson
Copy link
Contributor Author

Re-working this to remove usage of Downloader entirely per convo in Discord.

@AlexDickerson AlexDickerson marked this pull request as draft December 28, 2024 22:50
@AlexDickerson
Copy link
Contributor Author

Reworked this with a new approach. Abandoning and creating new PR

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