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

Optimize AddMasteryEffectOptionsToNode, improve startup time #6607

Conversation

Lancej
Copy link
Contributor

@Lancej Lancej commented Sep 6, 2023

Improves startup time by about 400 ms.

AddMasteryEffectOptionsToNode was combining all mastery option mod lines into one node.sd and then calling ProcessStats which led to many mod lines being combined in an attempt to parse them. These combined lines shouldn't be combined for parsing since they come from different mastery options and they also weren't cached so parsing them was very slow.

This change adds a new startIndex parameter to ProcessStats so it can incrementally parse the mastery options as they are added by AddMasteryEffectOptionsToNode instead of parsing them all at once.

Also added new "Startup time: #### ms" print message that measures from start until the first OnFrame call finishes. I was measuring just Main:Init before but there is some init code in the first OnFrame call so now I'm measuring until end of first frame.

Steps taken to verify a working solution:

  • Allocating / deallocating mastery nodes still behaves the same
  • "Show Node Power" still shows same results
  • "Search:" box on passive tree still highlights mastery nodes when it should

@LocalIdentity LocalIdentity added the enhancement New feature, calculation, or mod label Sep 7, 2023
@LocalIdentity LocalIdentity merged commit 0f654da into PathOfBuildingCommunity:dev Sep 7, 2023
@Lancej Lancej deleted the AddMasteryEffectOptionsToNode branch September 9, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, calculation, or mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants