Preload all extension types from include workflows #1890
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PathogenDavid
previously approved these changes
Jul 10, 2024
Significant groundwork was required to avoid dependencies on the instance field. Mostly this involved making all property serializer methods static.
|
@PathogenDavid I ended up doing one final refactor to defer workflow assignment as you suggested in #824. Turns out the fix required significant groundwork to avoid depending on The refactoring in itself is definitely an improvement. I was a bit worried something might break, but thankfully the most complicated infrastructure was already in place and we have some test coverage of some of the most subtle edge cases which got triggered and fixed. I have also tested usage with some fairly complex workflows and everything seems to work fine. |
PathogenDavid
requested changes
Jul 10, 2024
PathogenDavid
approved these changes
Jul 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Include workflows are loaded lazily and on-demand during the workflow build step, which means extension type discovery is performed in step-fashion for a newly loaded workflow. This gradual type discovery has dramatic implications in performance as documented in #1680, and this was having a visible impact on editor loading times, especially for complex applications with large numbers of include workflows.
This PR aims to improve the performance by preloading all extension types from included workflows ahead of time. The serialization of workflow properties was also optimized to minimize the number of required serializer instances.
The preloading optimization is disabled during builds to avoid supra-linear costs when recursively loading included workflows with deeply nested self-repeating motifs where the same workflow might be referenced multiple times.
Fixes #1680
Fixes #824