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 LocalModCard performance #1206

Merged
merged 2 commits into from
Feb 15, 2024
Merged

Improve LocalModCard performance #1206

merged 2 commits into from
Feb 15, 2024

Conversation

anttimaki
Copy link
Collaborator

No description provided.

@anttimaki
Copy link
Collaborator Author

anttimaki commented Feb 8, 2024

Some manual performance testing with a profile that contains 282 mods. Numbers are in seconds and are averages from three rounds, except for the the built column, which was checked only once. The first three columns use the dev build by created by yarn run run.

develop mod-typing PR PR built
Initial list render 13,5 17,3 17,4 14,3
Open disable modal 5,7 5,6 0 ‡ 0 ‡
Disable only BepInEx 25,4 22,5 14,1 12,3
Enable BepInEx 9,8 9,9 9,8 8,5
Open uninstall modal 4,5 4,6 0 ‡ 0 ‡
Uninstall only BepInEx 22,0 17,5 10,2 8,5
Reinstall BepInEx 11,6 11,1 11,2 10,0
Disable BepInEx & dependants N/A † N/A † 46,9 32,3

‡ = modal opens up practically instantly
† = Takes ridiculous amount of time. Had this run for 16 minutes while on lunch and it didn't finish in that time. Also the background update of package list gets triggered every 5 minutes, which slows both processes down even more

Summary: the changes depend on earlier branches in this stack, which slow down the initial rendering a bit. As a tradeoff it significantly improves performance of some operations. Staring just the numbers can be a bit misleading though, since all use cases include the initial render, while someone disabling BepInEx and all the mods depending on it seems an unlikely scenario. On the other hand initial rendering is done only once, while user might e.g. open the modals multiple times in a session.

src/components/views/LocalModList/LocalModCard.vue Outdated Show resolved Hide resolved
Copy link
Collaborator

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Base automatically changed from mod-typing to develop February 15, 2024 11:00
- Skip the whole process if the mod has no dependencies
- Combine the methods to avoid looping over the same localModList and
  dependency list multiple times
- Only loop over the localModList (which is the larger of arrays) once
  and return early if all dependencies are found
- Stop appending dash to mod names, it's no longer needed since the new
  implementation doesn't use .startsWith()
- Stop lowercasing the mod names since it doesn't seem to be needed.
  This is more about code clarity than performance.
@anttimaki anttimaki merged commit d3542e8 into develop Feb 15, 2024
7 checks passed
@anttimaki anttimaki deleted the mod-card-dependencies branch February 15, 2024 11:16
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.

3 participants