Skip to content

Refactor memory management and improve platform compatibility for Windows #4

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
34 changes: 18 additions & 16 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
[package]

name = "poule"
version = "0.3.2"
license = "MIT"
authors = ["Carl Lerche <[email protected]>", "Geoffroy Couprie <[email protected]>"]
description = "A pool of reusable values"
name = "poule"
version = "0.3.2"
license = "MIT"
authors = ["Carl Lerche <[email protected]>", "Geoffroy Couprie <[email protected]>"]
description = "A pool of reusable values"
documentation = "https://docs.rs/poule"
homepage = "https://github.com/sozu-proxy/poule"
repository = "https://github.com/sozu-proxy/poule"
readme = "README.md"
keywords = ["pool"]
edition = "2018"
exclude = [
".gitignore",
".travis.yml",
"deploy.sh",
"test/**/*",
homepage = "https://github.com/sozu-proxy/poule"
repository = "https://github.com/sozu-proxy/poule"
readme = "README.md"
keywords = ["pool"]
edition = "2018"
exclude = [
".gitignore",
".travis.yml",
"deploy.sh",
"test/**/*",
]

[dependencies]
libc = "0.2"

[target.'cfg(windows)'.dependencies]
windows-sys = { version = "0.48", features = ["Win32_Foundation", "Win32_System_Memory"] }

[[test]]
name = "test"
path = "test/test.rs"
Expand Down
26 changes: 11 additions & 15 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,15 +311,14 @@ impl<T> PoolInner<T> {

debug_assert!(nxt <= self.init, "invalid next index: {}", idx);

let res = self.next.compare_and_swap(idx, nxt, Ordering::Relaxed);

if res == idx {
break;
match self.next.compare_exchange(idx, nxt, Ordering::Relaxed, Ordering::Relaxed) {
Ok(_) => break,
Err(res) => {
// Re-acquire the memory before trying again
atomic::fence(Ordering::Acquire);
idx = res;
}
}

// Re-acquire the memory before trying again
atomic::fence(Ordering::Acquire);
idx = res;
}

self.used.fetch_add(1, Ordering::Relaxed);
Expand All @@ -328,7 +327,7 @@ impl<T> PoolInner<T> {

fn checkin(&self, ptr: *mut Entry<T>) {
let idx;
let mut entry: &mut Entry<T>;
let entry: &mut Entry<T>;

unsafe {
// Figure out the index
Expand All @@ -344,13 +343,10 @@ impl<T> PoolInner<T> {
// Update the entry's next pointer
entry.next = nxt;

let actual = self.next.compare_and_swap(nxt, idx, Ordering::Release);

if actual == nxt {
break;
match self.next.compare_exchange(nxt, idx, Ordering::Release, Ordering::Relaxed) {
Ok(_) => break,
Err(actual) => nxt = actual,
}

nxt = actual;
}
self.used.fetch_sub(1, Ordering::Relaxed);
}
Expand Down
123 changes: 102 additions & 21 deletions src/mmap.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
use std::{
ops::{Deref, DerefMut, Drop},
ptr, slice,
ops::{Deref, DerefMut},
slice,
};

#[cfg(unix)]
use libc::{
mmap, mprotect, munmap, MAP_ANON, MAP_FAILED, MAP_PRIVATE, PROT_NONE, PROT_READ, PROT_WRITE,
};

#[cfg(windows)]
use windows_sys::Win32::System::Memory::{
VirtualAlloc, VirtualFree, VirtualProtect,
MEM_COMMIT, MEM_RELEASE, MEM_RESERVE,
PAGE_NOACCESS, PAGE_READWRITE,
};

/// Memory map backend for the pool
///
/// This acts like a buffer that we can grow without changing its address
Expand All @@ -26,13 +34,77 @@ pub struct GrowableMemoryMap {
size: usize,
}

#[cfg(windows)]
impl GrowableMemoryMap {
pub fn new(capacity: usize) -> Result<Self, &'static str> {
let capacity = page_size(capacity);

let ptr = unsafe {
VirtualAlloc(
std::ptr::null_mut(),
capacity,
MEM_RESERVE,
PAGE_NOACCESS,
)
};

if ptr.is_null() {
return Err("could not map memory");
}

Ok(GrowableMemoryMap {
ptr: ptr as *mut u8,
capacity,
size: 0,
})
}

pub fn grow_to(&mut self, size: usize) -> Result<(), &'static str> {
let size = page_size(size);

if size <= self.size {
return Ok(());
}

if size > self.capacity {
return Err("new size cannot be larger than max capacity");
}

unsafe {
// Commit the memory and change protection
if VirtualAlloc(
self.ptr as _,
size,
MEM_COMMIT,
PAGE_READWRITE,
).is_null() {
return Err("could not commit memory");
}

let mut old_protect = 0;
if VirtualProtect(
self.ptr as _,
size,
PAGE_READWRITE,
&mut old_protect,
) == 0 {
return Err("could not change permissions on memory");
}
}

self.size = size;
Ok(())
}
}

#[cfg(unix)]
impl GrowableMemoryMap {
pub fn new(capacity: usize) -> Result<Self, &'static str> {
let capacity = page_size(capacity);

let ptr = unsafe {
mmap(
ptr::null_mut(),
std::ptr::null_mut(),
capacity,
PROT_NONE,
MAP_ANON | MAP_PRIVATE,
Expand All @@ -52,18 +124,6 @@ impl GrowableMemoryMap {
})
}

pub fn ptr(&self) -> *mut u8 {
self.ptr
}

pub fn size(&self) -> usize {
self.size
}

pub fn capacity(&self) -> usize {
self.capacity
}

pub fn grow_to(&mut self, size: usize) -> Result<(), &'static str> {
let size = page_size(size);

Expand All @@ -84,6 +144,13 @@ impl GrowableMemoryMap {
}
}

// Common implementations for both platforms
impl GrowableMemoryMap {
pub fn ptr(&self) -> *mut u8 {
self.ptr
}
}

impl Deref for GrowableMemoryMap {
type Target = [u8];

Expand All @@ -98,23 +165,37 @@ impl DerefMut for GrowableMemoryMap {
}
}

#[cfg(unix)]
impl Drop for GrowableMemoryMap {
fn drop(&mut self) {
unsafe {
if munmap(self.ptr as _, self.capacity) != 0 {
println!("could not unmap");
}
}
}
}

#[cfg(windows)]
impl Drop for GrowableMemoryMap {
fn drop(&mut self) {
let res = unsafe { munmap(self.ptr as _, self.capacity) };
if res != 0 {
println!("could not unmap");
unsafe {
if VirtualFree(self.ptr as _, 0, MEM_RELEASE) == 0 {
println!("could not unmap");
}
}
}
}

pub fn page_size(data_len: usize) -> usize {
let count = data_len / 0x1000;
let rem = data_len % 0x1000;
let page_size = if cfg!(windows) { 0x1000 } else { 0x1000 };
let count = data_len / page_size;
let rem = data_len % page_size;

if rem == 0 {
data_len
} else {
(count + 1) * 0x1000
(count + 1) * page_size
}
}

Expand Down
10 changes: 1 addition & 9 deletions test/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,6 @@ impl Vector {
self.0.length -= 1;
self.0.extra()[self.0.length]
}

fn len(&self) -> usize {
self.0.length
}

fn capacity(&self) -> usize {
self.0.extra().len()
}
}

struct MyPool {
Expand Down Expand Up @@ -87,7 +79,7 @@ pub fn test_extra_bytes() {
}

let mut v = Vec::new();
for i in 0..1023 {
for _i in 0..1023 {
v.push(pool.get().unwrap());
}

Expand Down