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

Pr/various mem optimizations #85

Conversation

wrenge
Copy link
Contributor

@wrenge wrenge commented Nov 26, 2024

This is a big one. It reduces allocations all around the code.

{
private ArrayPool<T> _owner;
private T[] _array;
private readonly RentIdGen _rentIdGen;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tricky one. Since RcRentedArray now a value type, it will copy values. That means when we dispose it, all copies will remain reference to owner and array.
To fix that we introduce rent id. When rent array, get vacant id and generation. On dispose check the generation. If generation is different, that means this rent was already freed. Otherwise release array and increase the generation.

@@ -55,7 +55,7 @@ private void Balance()
{
if (_dirty)
{
_items.Sort(_comparer); // reverse
_items.Sort(_comparison); // reverse
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List.Sort(IComparer) allocates Comparison, so we pass delegate directly to avoid allocations.

@wrenge
Copy link
Contributor Author

wrenge commented Nov 26, 2024

Feel free to ask questions if changes or commits are too confusing. PR is big so I recommend to look commit by commit.

@wrenge wrenge mentioned this pull request Nov 26, 2024
@ikpil
Copy link
Owner

ikpil commented Dec 3, 2024

Thank you for your contribution.
I’ll review it and mention you after merging it into another branch!
@wrenge

@ikpil ikpil changed the base branch from main to pr/wrenge-pr/various-mem-optimizations December 3, 2024 04:35
@ikpil ikpil merged commit d37a9c6 into ikpil:pr/wrenge-pr/various-mem-optimizations Dec 3, 2024
2 checks passed
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