Dynamo icon indicating copy to clipboard operation
Dynamo copied to clipboard

Remove dead code with big performance impact

Open dimven-adsk opened this issue 9 months ago • 0 comments

Purpose

There's a piece of code that gets evaluated every time you move the mouse and currently does effectively nothing. It does nothing because the NodeView's template was modified and the end condition is never met. The mouse will never hover over a port, because the ports are always covered by the hover indicator border (**transparent backgrounds are effectively solid for hit tests):

image

The intended logic per the comments was to detect when the mouse is over a connected out port and display an "arc_remove.cur" cursor. Per the code, it seems the logic changed at some point to instead display an "arc_select.cur" cursor when over an input port. Either way, none of this happens and as mentioned above this code now is completely useless. Yet we still pay the cost during graph navigation, which can easily add up over time and eat away resources from elsewhere. After navigating a large graph for just a few minutes, the total CPU time can easily grow to 5-10% of the total:

RwARVKvimh

EtPF3kAVp4

I expect that if you keep working with the graph continuously for a long time, the overall percentage will craw up even higher than that.

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
  • [x] 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)

(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

dimven-adsk avatar Feb 13 '25 17:02 dimven-adsk