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

Improved RawList API, Implementation and Tests #830

Merged
merged 8 commits into from
Jun 3, 2020
Merged

Conversation

ilexp
Copy link
Member

@ilexp ilexp commented May 30, 2020

As noted in #820, the RawList<T> class can benefit from newly available ref-returns in C# 7.3 for its indexer implementation. Since this is a breaking change for v4.0, it also was a good opportunity to rework and extend some of its API to better address its core use case.

This PR adds several small improvements, as well as a by-ref indexer for RawList<T>. For a detailed list, see the individual commits, which have been structured so each one represents a group of changes.

ilexp added 6 commits May 30, 2020 16:05
#CHANGE: RawList<T> now provides a by-ref indexer instead of the previous by-value indexer.
#REMOVE: Removed method overloads from RawList<T> that accept IEnumerable<T> to avoid suggesting an efficient implementation.
#CHANGE: Tweaked implementations of RawList<T> Add and Reserve methods to improve performance.
#ADD: Added RawList<T> methods RemoveLast, RemoveAtFast and RemoveRangeFast to allow more efficient removal in specific scenarios.
#CHANGE: Replaced custom RawList<T> FunctorComparer with the default Comparer<T>.Create helper.
@ilexp ilexp added Task ToDo that's neither a Bug, nor a Feature Core Area: Duality runtime or launcher Usability Related to API and UI usability Performance Related to runtime or editor performance labels May 30, 2020
@ilexp ilexp added this to the 4.0 milestone May 30, 2020
@ilexp ilexp requested review from SirePi and Barsonax May 30, 2020 14:29
@Barsonax
Copy link
Member

Barsonax commented Jun 1, 2020

For the performance improvements did you do any testing (benchmarkdotnet?)? Pretty sure this improves the performance but its always good to verify and also know how much it improves it.

@ilexp
Copy link
Member Author

ilexp commented Jun 1, 2020

Addressed the change request you made, see above.

For the performance improvements did you do any testing (benchmarkdotnet?)? Pretty sure this improves the performance but its always good to verify and also know how much it improves it.

You're right, but no, I didn't run any benchmarks. Mostly for time reasons and given the very small scope of the two tweaks, I stopped at "pretty sure it's an improvement".

@Barsonax
Copy link
Member

Barsonax commented Jun 1, 2020

Addressed the change request you made, see above.

For the performance improvements did you do any testing (benchmarkdotnet?)? Pretty sure this improves the performance but its always good to verify and also know how much it improves it.

You're right, but no, I didn't run any benchmarks. Mostly for time reasons and given the very small scope of the two tweaks, I stopped at "pretty sure it's an improvement".

Ok well as the changes are pretty small and seem to make sense iam assuming it will be ok :)

@ilexp ilexp merged commit 4cff3d8 into master Jun 3, 2020
@ilexp ilexp deleted the feature/rawlist-update branch June 3, 2020 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Area: Duality runtime or launcher Performance Related to runtime or editor performance Task ToDo that's neither a Bug, nor a Feature Usability Related to API and UI usability
Development

Successfully merging this pull request may close these issues.

2 participants