Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace ArrayList/ICollection used for changelog in CollectionView with generic List #9853

Merged
merged 4 commits into from
Dec 31, 2024

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Sep 27, 2024

Description

Replaces ArrayList used for tracking changes with List<NotifyCollectionChangedEventArgs>. Instead of using SyncRoot (object) of the ArrayList we will just use the new Lock for clarity and slightly improved performance.

Also modifies ProcessChangeLog / DeferProcessing from ICollection to List<NotifyCollectionChangedEventArgs> for improved performance.

  • The ProcessChangeLog null-check was there because of the as casting pattern, if the args inserted into the _changeLog would ever be null, it would blow before it was even added in PostChange (the only proc adding into _changeLog).
  • DeferProcessing doesn't need the null-check branch because the only time _changeLog is written into is always covered using _changeLogLock, so it cannot be NULL, and the ref transfer to _tempChangeLog is covered in lock too (ProcessInvoke).
  • startCount variable was removed because it was unused.

Customer Impact

Improved performance when updating the CollectionView, clearer code for developers.

Regression

No.

Testing

Local build, testing with CollectionView sample apps, updating from background thread.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@dipeshmsft dipeshmsft merged commit 43eb4e6 into dotnet:main Dec 31, 2024
8 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Community Contribution A label for all community Contributions Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants