Dynamo icon indicating copy to clipboard operation
Dynamo copied to clipboard

collection performance

Open pinzart90 opened this issue 3 years ago • 3 comments

Purpose

Improve performance when opening graphs https://jira.autodesk.com/browse/DYN-5005

Changes: Performance improvement when dealing with iterations

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

Release Notes

(FILL ME IN) Brief description of the fix / enhancement. 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

pinzart90 avatar Jun 07 '22 15:06 pinzart90

Curious if all the locking and unlocking for each and every smart collection operation has any performance impact? It looks like in most cases, it may be overkill to make it thread safe so aggressively if that's the case, especially if such cross-threading scenarios will be rare.

The SmartObservableCollection is not meant to be a replacement for all Lists/dictionaries or for highly sensitive areas. It should mostly be used for WPF bindings (and optionally when dealing with multiple threads). As an example: Scheduler thread: Model.smartObservableItems => changed when running a graph UI thread: View => BindTo Model.smartObservableItems

SmartObservableCollection is already executing a lot of instructions on every collection operation (ex. property change notifications, collection change notifications). In the scenario where there are only concurrent reads, adding locks to the mix should not impact performance in a noticeable way. In the scenario where there are concurrent reads and writes, obviously one of the operations will wait for the other with some performance penalty. I can run performance tests to confirm.

pinzart90 avatar Jun 28 '22 03:06 pinzart90

Idea for more improvement is to DelayGraphExecution when calling DynamoModel::GetConnectorsToAddAndDelete. At this time...when hooking up a new node to an already connected node, Dynamo creates and schedules 2 UpdateGraphAsyncTasks.

pinzart90 avatar Dec 01 '22 15:12 pinzart90

Idea for more improvement is to DelayGraphExecution when calling DynamoModel::GetConnectorsToAddAndDelete. At this time...when hooking up a new node to an already connected node, Dynamo creates and schedules 2 UpdateGraphAsyncTasks.

see this issue for something related, it might be a slightly different case https://jira.autodesk.com/browse/DYN-5128

mjkkirschner avatar Dec 01 '22 16:12 mjkkirschner