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

fix: fix feature "alloc_trait" to adapt to the current Rust Allocator API #37

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MrCroxx
Copy link
Member

@MrCroxx MrCroxx commented Aug 30, 2022

The alloc_trait feature cannot be compiled because it used deprecated API core::alloc::{Alloc, Excess}.

Status:

  • Make alloc_trait compilable.
  • Restore benchmark.
  • Remove outdated tests.

TODO (may be not in this PR):

  • Add tests for grow and shrink.
  • Add benchmark for grow and shrink.

@MrCroxx
Copy link
Member Author

MrCroxx commented Aug 30, 2022

Temporarily close. I'll reopen it when my fork repo is stable.

unsafe fn alloc(&mut self, layout: Layout) -> Result<NonNull<u8>, AllocErr> {
NonNull::new(GlobalAlloc::alloc(self, layout)).ok_or(AllocErr)
fn alloc_impl(&self, layout: Layout, zeroed: bool) -> Result<NonNull<[u8]>, AllocError> {
match layout.size() {
Copy link
Member

Choose a reason for hiding this comment

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

Better use if/else and check non-zero first.

let raw_ptr = GlobalAlloc::realloc(self, ptr.as_ptr(), old_layout, new_size);
let ptr = NonNull::new(raw_ptr).ok_or(AllocError)?;
if zeroed {
raw_ptr.add(old_size).write_bytes(0, new_size - old_size);
Copy link
Member

Choose a reason for hiding this comment

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

Should use realloc(ptr, flags | MALLOCX_ZERO.

) -> Result<NonNull<u8>, AllocErr> {
NonNull::new(GlobalAlloc::realloc(self, ptr.as_ptr(), layout, new_size)).ok_or(AllocErr)
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
self.alloc_impl(layout, false)
Copy link
Member

Choose a reason for hiding this comment

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

As these are not complicated functions, Instead of extracting an impl method, implementing them in the function directly seems more clear.

if layout.size() != 0 {
// SAFETY: `layout` is non-zero in size,
// other conditions must be upheld by the caller
unsafe { GlobalAlloc::dealloc(self, ptr.as_ptr(), layout) }
Copy link
Member

Choose a reason for hiding this comment

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

You can call self.dealloc directly.

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