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

Get Units Only Once Per Execution #6

Open
wants to merge 1 commit into
base: reope/performance-base
Choose a base branch
from

Conversation

dimven-adsk
Copy link
Owner

@dimven-adsk dimven-adsk commented Sep 4, 2024

List of Affected Nodes/Modules

All nodes that deal with geometry
All nodes that get or set parameters with units

Current Performance

Every time we transfer geometry from Dynamo to Revit, or the other way around, we have to first get the current document's units and figure out the conversion ratio. Getting the document units has a non-trivial cost and currently we are fetching them multiple times for every single geometric conversion (once per point, twice per line, multiple times per spline/face/solid, etc. ). Further more, we have to perform a similar conversion when setting parameter values with units, because all parameters in Revit have an internal representation.

devenv_PGtUErrLnK

Proposed Performance

If we cache the units at the start of the graph execution, and clear the cache at the end of the graph execution, we can completely remove this performance penalty.

Currently Dynamo does not distribute any nodes that can modify the project units. I am also not aware of any custom packages that would do so. I feel it is extremely unlikely that a user would ever change the project units in the middle of a graph operation. If a case like this ever arises, we can document how to clear the units cache.

If the user changes units through standard means (inside the Revit UI) between two executions, everything should work as before because the cache will be cleared and re-populated.

Kudos to the team that implemented the UnitConverter. They did an excellent job because all unit conversions are piped through a single method, which makes implementing the cache mechanism very simple :)

devenv_sDUQggU9U1

Dynamo Tuneup Comparison

With a very basic graph that creates 6000 instances by point and then gets their location [1], we see about 2450ms of TuneUp time [2] before and about 2180ms after:

Revit_qZPCtzOX0Z
Revit_FoqsvkhBc0

The more geometry and parameters a graph deals with, the greater the benefit of caching the units will be.

Checklist

  • There are no public function signature changes
  • Any public code that is no longer in use is tagged as obsolete and preserved.
  • The code changes have been documented
  • The overall behavior does not change

[1] In a realistic scenario, you should have a document regeneration before getting a new instance's location
[2] This as mentioned does not include the document regeneration, which is irrelevant to the comparison

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.

2 participants