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 addonCacheKey computation #1102

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

SergeAstapov
Copy link
Collaborator

Since adoption of ember-auto-import v2 and ecosystem migration to @embroider/[email protected] we seen significant performance degradation in our not that small app - it went from 4 mins to 20 mins (but with way to inefficient dependencies graph and too many cross-dependencies, which is separate problem).

Debugging and profiling leads to gatherAddonCacheKey function being biggest contributor.

First issue
Every time @embroider/macros addon is included (and it's included way more than few times due to ember-auto-import and many other addons depend on it) the whole tree travels happens over and over.

Second issue
there is return [...memo].join('|') line which runs for each end every recursion level but the result is used only from top-level return when invoked with project as an argument.

Changes in this PR propose to traverse addons three only once and build cacheKey string only once per project.

I wasn't sure if project always is the same reference or there are use cases when we may have more than one project during build time hence used WeakMap.

If we know for sure project always is the same reference - than we can replace WeakMap with simple string variable that would be returned once we invoke gatherAddonCacheKey second and subsequent times.

@ef4
Copy link
Contributor

ef4 commented Feb 5, 2022

Thanks, very nice.

@ef4 ef4 merged commit 9bc26f2 into embroider-build:main Feb 5, 2022
@SergeAstapov SergeAstapov deleted the optimize-addon-cache-key branch February 5, 2022 21:20
@rwjblue rwjblue added the bug Something isn't working label Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants