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

Eliminate allocations #43

Merged
merged 15 commits into from
Jul 7, 2018
Merged

Eliminate allocations #43

merged 15 commits into from
Jul 7, 2018

Conversation

iamcarbon
Copy link
Contributor

@iamcarbon iamcarbon commented Jun 11, 2018

What are your thoughts on converting all the classes to readonly structs to eliminate allocations in the library?

I've started a PR to see what this would look like. The API would stay the same although any library depending on things would need to recompile.

This could also be a good time to increase the library to V2 and drop support for .NET 4.0.

@tompazourek
Copy link
Owner

This looks like a good idea to me. I've actually originally started building the library with structs instead of classes, but now when we have the readonly structs, I think it might be an opportunity to reconsider.

I haven't got much free time right now to checkout the fork and get a closer look, but I'd like to try it out in the following weeks (weekends/evenings). I haven't yet had a chance to play with any readonly structs, and I think it's a good chance to explore it. In general, I am all in favor of immutability, especially when the compiler can help with it and utilize it as well...

Thanks!

@iamcarbon
Copy link
Contributor Author

iamcarbon commented Jun 14, 2018

I believe this covers everything. All tests continue to pass with no changes.

@iamcarbon iamcarbon changed the title [WIP] Eliminate allocations Eliminate allocations Jun 14, 2018
@tompazourek tompazourek merged commit b4cbb47 into tompazourek:master Jul 7, 2018
@tompazourek
Copy link
Owner

tompazourek commented Jul 7, 2018

Merged the PR today, sorry it took a while.

I ended up dropping the IRGB interface for now. I've also done some small code formatting cleanup (mostly using expression-bodied members more).

I just published this as version 2.0.0 on NuGet now.

Thanks again for the great work.

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.

None yet

2 participants