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

Align heap to 64 bytes #119

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Align heap to 64 bytes #119

merged 2 commits into from
Oct 18, 2024

Conversation

jjyr
Copy link
Collaborator

@jjyr jjyr commented Oct 15, 2024

jjyr/buddy-alloc#18 (comment)

buddy-alloc 0.5 uses ptr#write_aligned which costs more cycles than write.

In 0.6 we assume the heap memory is aligned to 64 bytes, so we can switch to ptr::write.

@jjyr jjyr requested review from mohanson and XuJiandong October 15, 2024 13:16

static mut _BUDDY_HEAP: _AlignedHeap<$heap_size> = _AlignedHeap([0u8; $heap_size]);
static mut _FIXED_BLOCK_HEAP: _AlignedHeap<$fixed_block_heap_size> =
_AlignedHeap([0u8; $fixed_block_heap_size]);

#[global_allocator]
static ALLOC: $crate::buddy_alloc::NonThreadsafeAlloc = unsafe {
let fast_param = $crate::buddy_alloc::FastAllocParam::new(
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's suggested to insert a checking function here to make sure all ptr are aligned with 64.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If HEAP is aligned to 64 bytes, buddy-alloc allocated ptrs are aligned to 64.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Previously, the reported error was misleading and made debugging very difficult. If the error occurs due to incorrect configuration, troubleshooting would be much more straightforward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. We already have a debug assertion https://github.com/jjyr/buddy-alloc/blob/master/src/fast_alloc.rs#L116
  2. But unfortunately, we can't trigger a panic or assert from the global allocator, because the panic itself depends on the allocator, I believe the reported error is triggerred by this assertion.
  3. #[repr(align(64))] is guaranteed by the compiler, it is not exposed to ckb-std users, so they have no chance t mis-configure this.

@jjyr jjyr merged commit 605c3c2 into master Oct 18, 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.

3 participants