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

Improve performance of installing mods with multiple dependencies #1505

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

anttimaki
Copy link
Collaborator

After the downloads are completed, the download modal hangs at 100% for
a moment while mods are copied to the profile folder and the mods.yml
file is updated.

The old implementation called ProfileModList.addMod() for each mod,
which meant that for each mod:

  • The mods.yml was read six times, including checking if the file
    exists, reading it, parsing the yaml into ManifestV2[], and checking
    if each mod in the profile has icon file inside the profile folder
  • The mods.yml was written twice, serializing the ManifestV2[] into
    yaml.
  • For a modpack with 69 mods, all already in the cache, this took 25
    seconds

The new implementation tracks the contents of the ManifestV2[] in
memory. As a result mods.yml is read twice and saved once regardless
of the number of processed mods. For the aforementioned modpack this
takes four seconds.

It's worth noting that the implementations aren't completely identical
in other things. The new implementation doesn't necessarily define
which mod caused the error, whereas the old one did. The new
implementation doesn't give special treatment to bbepis-BepInExCack
anymore - it's now uninstalled like all the other files are.

Unfortunately this doesn't allow us to use the new implementation
with the static DownloadModModal.downloadSpecific(), nor clean up
the old implementation. That would require more refactoring, which
doesn't need to block these changes.

After the downloads are completed, the download modal hangs at 100% for
a moment while mods are copied to the profile folder and the mods.yml
file is updated.

The old implementation called ProfileModList.addMod() for each mod,
which meant that for each mod:

- The mods.yml was read six times, including checking if the file
  exists, reading it, parsing the yaml into ManifestV2[], and checking
  if *each* mod in the profile has icon file inside the profile folder
- The mods.yml was written twice, serializing the ManifestV2[] into
  yaml.
- For a modpack with 69 mods, all already in the cache, this took 25
  seconds

The new implementation tracks the contents of the ManifestV2[] in
memory. As a result mods.yml is read twice and saved once regardless
of the number of processed mods. For the aforementioned modpack this
takes four seconds.

It's worth noting that the implementations aren't completely identical
in other things. The new implementation doesn't necessarily define
which mod caused the error, whereas the old one did. The new
implementation doesn't give special treatment to bbepis-BepInExCack
anymore - it's now uninstalled like all the other files are.

Unfortunately this doesn't allow us to use the new implementation
with the static DownloadModModal.downloadSpecific(), nor clean up
the old implementation. That would require more refactoring, which
doesn't need to block these changes.
Copy link
Collaborator

@VilppeRiskidev VilppeRiskidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 👍

Base automatically changed from profilemodlist-icon to develop October 30, 2024 07:30
@anttimaki anttimaki merged commit 1e70fa6 into develop Oct 30, 2024
5 checks passed
@anttimaki anttimaki deleted the install-mods-disk-usage branch October 30, 2024 07:30
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