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

Combined XAML / Optimized resource loading #15795

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

dimven-adsk
Copy link
Contributor

@dimven-adsk dimven-adsk commented Jan 31, 2025

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:

Using one large XAML file not only makes it easier to consume, but can also drastically improving loading performance.

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

Current Performance Combined XAML Performance
devenv_ehOV8Vqua3 22sR9ltSNd

By merging the dictionaries, we reduce the component loading time in heavy resources like the node view and eliminate the resX lookups.

Initial Layout

Current Performance Combined XAML Performance
JIcMkKmSQK devenv_FMs03Pxx7n

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

Current Performance Combined XAML Performance
devenv_fKyhG1iMMi devenv_7deUZPIlrU
devenv_Q1KlVBX36v devenv_StU3vgOabL

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:
devenv_o967CRw5zT

Combined XAML:
DynamoSandbox_F7O1WnnauH

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants