-
Notifications
You must be signed in to change notification settings - Fork 638
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
Combined XAML / Optimized resource loading #15795
Open
dimven-adsk
wants to merge
22
commits into
DynamoDS:master
Choose a base branch
from
dimven-adsk:reope/performance-combined-xaml
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Combined XAML / Optimized resource loading #15795
dimven-adsk
wants to merge
22
commits into
DynamoDS:master
from
dimven-adsk:reope/performance-combined-xaml
+1,418
−1,614
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
Reope/performance combined xaml
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.
Purpose
The goal of this PR is to greatly simplify the code-base, and organize all of the visual resources into a single file that is loaded and parsed only once at the application level. We use the XAMLTools task to achieve this. As per the tool's documentation:
What this means, is that XAML parsing for heavy components like the node view, the annotation view and others can be removed completely and layouting can be streamlined (no need to look into multiple merged resource dictionaries or keep multiple duplicate reference definitions in memory).
Another benefit is that the code base is greatly reduced and you no longer have to manage resource dictionaries per each child view.
The downside is that we had to make huge modifications to the codebase (ended up with a very large changelog) and that external view extensions that reference the Dynamo Modern resources (like TuneUp) will have to be updated to point to the new combined XAML dictionary (should be very easy to do).
Profiling
Component Loading / Parsing
By merging the dictionaries, we reduce the component loading time in heavy resources like the node view and eliminate the resX lookups.
Initial Layout
It's been a challenge to get accurate and reproducible numbers for the profiling, because it is impossible to automate the UI interactions and to exactly replicate the clicks/actions (also because Visual Studio can be a total hog). The above demonstrates that the overall time on the initial layout of a large graph has been reduced
Navigational Layout
During the graph navigation, the expensive parsing of the shared dictionaries is removed and the large layout block is broken up into more manageable chunks. Notice how the area under the blue composition line is greatly reduced (we're closer to the 60FPS target and should have a smoother visual experience). I need to do more testing, but if we ignore the allocations, it looks like there is less pressure on the garbage collector because we have less items to recycle.
User Experience
Current:

Combined XAML:

Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
@ilianakp @mjkkirschner @pinzart90
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of