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

Add support for returning displaced entries for re-use. #22

Merged
merged 1 commit into from Aug 22, 2021

Conversation

Byron
Copy link
Contributor

@Byron Byron commented Aug 22, 2021

I added this to be able to avoid allocations if the entry is allocating, In case of gitoxide this saves a couple of millions of allocations in favour of a few thousand re-allocs.

Performance wasn't measurably better in my case, just as if the allocator can already repurpose previously freed slabs of memory pretty well or at only a very low cost.

Since it doesn't seem to hurt providing this capability, I am creating this PR anyway in the hopes it does indeed make sense to some in this form or another.

Please let me know :).

@Byron
Copy link
Contributor Author

Byron commented Aug 22, 2021

Here is how the altered method can be used.

@Byron Byron mentioned this pull request Aug 22, 2021
43 tasks
Byron added a commit to Byron/gitoxide that referenced this pull request Aug 22, 2021
@mbrubeck
Copy link
Collaborator

Thanks, this is great. I don't think it needs to be an optional feature, though. You can use core::mem::replace in no_std crates. Also, changing the signature of a function when a feature is enabled could technically break other crates in the same dependency graph that depend on the type of the function.

I'll merge this PR, and then push a follow-up to make it always use the new method.

@mbrubeck mbrubeck merged commit a9e01cd into servo:master Aug 22, 2021
@mbrubeck
Copy link
Collaborator

mbrubeck commented Aug 22, 2021

@Byron
Copy link
Contributor Author

Byron commented Aug 23, 2021

Absolutely awesome, thank you!

Also thanks for removing the feature toggle, there is so much I don't know about no-std, but I will get there (probably when gitoxide goes WASM :) ).

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