Skip to content

Conversation

dimven-adsk
Copy link
Contributor

Purpose

If you install the top 10 packages from the package manager, you'll end up with over 1300 DYFs in total. Every single one of those gets loaded in memory and processed not once but twice when reading its CustomNodeInfo. That can be avoided completely by creating a file stream and reading the json content line by line. The DYNs are usually pretty small in size, but when there's thousands of them, this can quickly add up. The optimization proposed here is very similar to the one in #15759 and brings the exact same benefits:

  1. Stream the files instead of reading their entire content - reduce memory and GC load.
  2. Skip the json validation (which forces us to pre-parse the file). It's not important why the file fails - Dynamo shouldn't be a json parser, just a json consumer. If the file is not a json or if the json content is malformed, you should use the right tool to figure that out.
  3. Read and store only the properties that are actually needed, which once again reduces the memory and GC load.

Current performance:

A lot of time is spent on reading and parsing the JSON data.

devenv_1tawChKnhD
devenv_VBr90HZHpv
devenv_PMax0803zF


Proposed performance:

By streaming the files, we can improve the performance of the DYF scan function more than twice.

devenv_XBrA28O0Yb
devenv_MIBF0dPgw2
devenv_ejbZzEPqDo

Declarations

Check these if you believe they are true

  • 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
  • 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
  • This PR introduces new feature code involve network connecting and is tested with no-network mode.

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)

(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

@QilongTang QilongTang requested review from a team and Copilot June 5, 2025 14:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces performance improvements for loading many DYF files by streaming JSON content rather than loading and parsing entire files into memory. Key changes include:

  • Using a new streaming JSON method in CustomNodeManager to retrieve custom node info.
  • Adding a GetFromJsonDocument static method in CustomNodeDefinition to efficiently extract only the required JSON properties.
  • Minor adjustments to CustomNodeInfo properties and constructors for improved deserialization.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/DynamoCore/Core/CustomNodeManager.cs Replaces full-file JSON parsing with streaming and simplifies error handling.
src/DynamoCore/Core/CustomNodeDefinition.cs Introduces a streaming JSON reader method and updates property initializations.

Copy link
Contributor

@RobertGlobant20 RobertGlobant20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

4 participants