-
-
Notifications
You must be signed in to change notification settings - Fork 350
freelist control #1603
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
Merged
freelist control #1603
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,97 @@ | ||||||
use std::cell::RefCell; | ||||||
use std::ops::{Deref, DerefMut}; | ||||||
|
||||||
/// A buffer that is returned to the free-list after usage. | ||||||
#[derive(Clone)] | ||||||
pub struct Buffer<'repo> { | ||||||
/// The buffer that would be returned to the freelist of `repo`. | ||||||
/// Note that buffers without capacity (i.e. without allocation) aren't returned. | ||||||
pub inner: Vec<u8>, | ||||||
/// The repository from whose free-list the `inner` buffer was taken, and to which it will be returned. | ||||||
pub repo: &'repo crate::Repository, | ||||||
} | ||||||
|
||||||
impl From<Buffer<'_>> for Vec<u8> { | ||||||
fn from(mut value: Buffer<'_>) -> Self { | ||||||
std::mem::take(&mut value.inner) | ||||||
} | ||||||
} | ||||||
|
||||||
impl Deref for Buffer<'_> { | ||||||
type Target = Vec<u8>; | ||||||
|
||||||
fn deref(&self) -> &Self::Target { | ||||||
&self.inner | ||||||
} | ||||||
} | ||||||
|
||||||
impl DerefMut for Buffer<'_> { | ||||||
fn deref_mut(&mut self) -> &mut Self::Target { | ||||||
&mut self.inner | ||||||
} | ||||||
} | ||||||
|
||||||
impl Drop for Buffer<'_> { | ||||||
fn drop(&mut self) { | ||||||
self.repo.reuse_buffer(&mut self.inner); | ||||||
} | ||||||
} | ||||||
|
||||||
/// Internal | ||||||
impl crate::Repository { | ||||||
/// Note that the returned buffer might still have data in it. | ||||||
#[inline] | ||||||
pub(crate) fn free_buf(&self) -> Vec<u8> { | ||||||
self.bufs | ||||||
.as_ref() | ||||||
.and_then(|bufs| bufs.borrow_mut().pop()) | ||||||
.unwrap_or_default() | ||||||
} | ||||||
|
||||||
/// This method is commonly called from the destructor of objects that previously claimed an entry | ||||||
/// in the free-list with [crate::Repository::free_buf]. | ||||||
/// They are welcome to take out the data themselves, for instance when the object is detached, to avoid | ||||||
/// it to be reclaimed. | ||||||
#[inline] | ||||||
pub(crate) fn reuse_buffer(&self, data: &mut Vec<u8>) { | ||||||
if data.capacity() > 0 { | ||||||
if let Some(bufs) = self.bufs.as_ref() { | ||||||
bufs.borrow_mut().push(std::mem::take(data)); | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/// Freelist configuration | ||||||
/// | ||||||
/// The free-list is an internal and 'transparent' mechanism for obtaining and re-using memory buffers when | ||||||
/// reading objects. That way, trashing is avoided as buffers are re-used and re-written. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
is this a typo? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wow, it wasn't a typo, but I realised now that it's not |
||||||
/// | ||||||
/// However, there are circumstances when releasing memory early is preferred, for instance on the server side. | ||||||
/// | ||||||
/// Also note that the free-list isn't cloned, so each clone of this instance starts with an empty one. | ||||||
impl crate::Repository { | ||||||
/// Return an empty buffer which is tied to this repository instance, and reuse its memory allocation by | ||||||
/// keeping it around even after it drops. | ||||||
pub fn empty_reusable_buffer(&self) -> Buffer<'_> { | ||||||
let mut inner = self.free_buf(); | ||||||
inner.clear(); | ||||||
Buffer { inner, repo: self } | ||||||
} | ||||||
|
||||||
/// Set the currently used freelist to `list`. If `None`, it will be disabled entirely. | ||||||
/// | ||||||
/// Return the currently previously allocated free-list, a list of reusable buffers typically used when reading objects. | ||||||
/// May be `None` if there was no free-list. | ||||||
pub fn set_freelist(&mut self, list: Option<Vec<Vec<u8>>>) -> Option<Vec<Vec<u8>>> { | ||||||
let previous = self.bufs.take(); | ||||||
self.bufs = list.map(RefCell::new); | ||||||
previous.map(RefCell::into_inner) | ||||||
} | ||||||
|
||||||
/// A builder method to disable the free-list on a newly created instance. | ||||||
pub fn without_freelist(mut self) -> Self { | ||||||
self.bufs.take(); | ||||||
self | ||||||
} | ||||||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this correct? I think otherwise the sentence is wonky
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! Would you be able to raise a new PR with the changes? Otherwise I can probably U-Boot the changes into the next PR.