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

[memory-0.2.1] prevent over-allocation #34

Merged
merged 4 commits into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Change Log

## memory-0.2.1 (12-10-2020)
- remove `hibitset` dependency
- fix overallocating memory when nothing is allocated yet
- fix overallocating memory after a few cycles of allocation

## memory-0.2, descriptor-0.2
- update to gfx-hal-0.6
- remove `colorful` dependency
Expand Down
3 changes: 1 addition & 2 deletions gfx-memory/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "gfx-memory"
version = "0.2.0"
version = "0.2.1"
authors = [
"omni-viral <[email protected]>",
"The Gfx-rs Developers",
Expand All @@ -18,7 +18,6 @@ description = "gfx-hal memory allocator"
[dependencies]
fxhash = "0.2"
hal = { package = "gfx-hal", version = "0.6" }
hibitset = { version = "0.6", default-features = false }
log = "0.4"
slab = "0.4"

Expand Down
130 changes: 118 additions & 12 deletions gfx-memory/src/allocator/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::{
AtomSize, Size,
};
use hal::{device::Device as _, Backend};
use hibitset::{BitSet, BitSetLike as _};
use std::{
collections::{BTreeSet, HashMap},
hash::BuildHasherDefault,
Expand Down Expand Up @@ -131,11 +130,101 @@ pub struct GeneralAllocator<B: Backend> {
unsafe impl<B: Backend> Send for GeneralAllocator<B> {}
unsafe impl<B: Backend> Sync for GeneralAllocator<B> {}

mod bit {
/// A hierarchical bitset hardcoded for 2 levels and only 64 bits.
#[derive(Debug, Default)]
pub struct BitSet {
mask: u64,
groups: u8,
}

impl BitSet {
const GROUP_SIZE: u32 = 8;

pub fn add(&mut self, index: u32) {
self.mask |= 1 << index;
self.groups |= 1 << (index / Self::GROUP_SIZE);
}

pub fn remove(&mut self, index: u32) {
self.mask &= !(1 << index);
let group_index = index / Self::GROUP_SIZE;
let group_mask = ((1 << Self::GROUP_SIZE) - 1) << (group_index * Self::GROUP_SIZE);
if self.mask & group_mask == 0 {
self.groups &= !(1 << group_index);
}
}

pub fn iter(&self) -> BitIterator {
BitIterator {
mask: self.mask,
groups: self.groups,
index: 0,
}
}
}

#[test]
fn test_bit_group() {
let mut bs = BitSet::default();
bs.add(13);
assert_eq!(bs.groups, 2);
bs.add(20);
assert_eq!(bs.groups, 6);
bs.add(23);
bs.remove(13);
assert_eq!(bs.groups, 4);
}

pub struct BitIterator {
mask: u64,
groups: u8,
index: u32,
}

impl BitIterator {
const TOTAL: u32 = std::mem::size_of::<super::Size>() as u32 * 8;
}

impl Iterator for BitIterator {
type Item = u32;
fn next(&mut self) -> Option<u32> {
let result = loop {
if self.index >= Self::TOTAL {
return None;
}
if self.index & (BitSet::GROUP_SIZE - 1) == 0
&& (self.groups & (1 << (self.index / BitSet::GROUP_SIZE))) == 0
{
self.index += BitSet::GROUP_SIZE;
} else {
if (self.mask & (1 << self.index)) != 0 {
break self.index;
}
self.index += 1;
}
};
self.index += 1;
Some(result)
}
}

#[test]
fn test_bit_iter() {
let mut bs = BitSet::default();
let bits = &[2u32, 5, 24, 39, 40, 41, 42, 62];
for &index in bits {
bs.add(index);
}
let collected = bs.iter().collect::<Vec<_>>();
assert_eq!(&bits[..], &collected[..]);
}
}

use bit::BitSet;

#[derive(Debug)]
struct SizeEntry<B: Backend> {
/// Total count of allocated blocks with size corresponding to this entry.
total_blocks: Size,

/// Bits per ready (non-exhausted) chunks with free blocks.
ready_chunks: BitSet,

Expand All @@ -147,12 +236,19 @@ impl<B: Backend> Default for SizeEntry<B> {
fn default() -> Self {
SizeEntry {
chunks: Default::default(),
total_blocks: 0,
ready_chunks: Default::default(),
}
}
}

impl<B: Backend> SizeEntry<B> {
fn next_block_count(&self, block_size: Size) -> u32 {
self.chunks.iter().fold(1u32, |max, (_, chunk)| {
(chunk.num_blocks(block_size) as u32).max(max)
}) * 2
}
}

/// A bit mask of block availability.
type BlockMask = u64;

Expand Down Expand Up @@ -294,6 +390,13 @@ impl<B: Backend> GeneralAllocator<B> {
// Allocate block for the chunk.
self.alloc_from_entry(device, chunk_size, 1, block_size)?
}
None if requested_chunk_size > self.min_device_allocation => {
// Allocate memory block from the device.
// Note: if we call into `alloc_block` instead, we are going to be
// going larger and larger blocks until we hit the ceiling.
let chunk = self.alloc_chunk_from_device(device, block_size, clamped_count)?;
return Ok((chunk, requested_chunk_size));
}
None => {
// Allocate a new block for the chunk.
self.alloc_block(device, requested_chunk_size, block_size)?
Expand Down Expand Up @@ -380,9 +483,9 @@ impl<B: Backend> GeneralAllocator<B> {
return Err(hal::device::OutOfMemory::Host.into());
}

// This is an estimated block count, and it's a hint.
// The estimated block count is a hint.
// The actual count will be clamped between MIN and MAX.
let estimated_block_count = size_entry.total_blocks as u32;
let estimated_block_count = size_entry.next_block_count(block_size);
let (chunk, allocated) = self.alloc_chunk(device, block_size, estimated_block_count)?;
log::trace!("\tChunk init mask: 0x{:x}", chunk.blocks);
let size_entry = self.sizes.entry(block_size).or_default();
Expand Down Expand Up @@ -428,9 +531,8 @@ impl<B: Backend> GeneralAllocator<B> {
align
);
let size_entry = self.sizes.entry(block_size).or_default();
size_entry.total_blocks += 1;

let overhead = (MIN_BLOCKS_PER_CHUNK as Size - 1) / size_entry.total_blocks;
let overhead = (MIN_BLOCKS_PER_CHUNK - 1) / size_entry.next_block_count(block_size);
if overhead >= 1 && block_size >= LARGE_BLOCK_THRESHOLD {
// this is chosen is such a way that the required `count`
// is less than `MIN_BLOCKS_PER_CHUNK`.
Expand All @@ -440,7 +542,7 @@ impl<B: Backend> GeneralAllocator<B> {
);
let chunk_size = match self
.chunks
.range(ideal_chunk_size..block_size * overhead)
.range(ideal_chunk_size..block_size * overhead as Size)
.find(|&size| size % align == 0)
{
Some(&size) => size,
Expand Down Expand Up @@ -631,10 +733,14 @@ impl<B: Backend> Chunk<B> {
start..end
}

fn num_blocks(&self, block_size: Size) -> Size {
let range = self.range();
((range.end - range.start) / block_size).min(MAX_BLOCKS_PER_CHUNK as Size)
}

/// Check if there are free blocks.
fn is_unused(&self, block_size: Size) -> bool {
let range = self.range();
let blocks = ((range.end - range.start) / block_size).min(MAX_BLOCKS_PER_CHUNK as Size);
let blocks = self.num_blocks(block_size);

let high_bit = 1 << (blocks - 1);
let mask = (high_bit - 1) | high_bit;
Expand Down
1 change: 1 addition & 0 deletions gfx-memory/src/allocator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ unsafe fn allocate_memory_helper<B: hal::Backend>(
) -> Result<(Memory<B>, Option<NonNull<u8>>), hal::device::AllocationError> {
use hal::device::Device as _;

log::trace!("Raw allocation of size {} for type {:?}", size, memory_type);
let raw = device.allocate_memory(memory_type, size)?;

let ptr = if memory_properties.contains(hal::memory::Properties::CPU_VISIBLE) {
Expand Down