From 66b0a097f48c5619a923d1ac5f48f5699481a1a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denizhan=20Dak=C4=B1l=C4=B1r?= <71423969+zelosleone@users.noreply.github.com> Date: Sat, 28 Dec 2024 20:11:18 +0300 Subject: [PATCH 1/2] Refactor memory management and improve platform compatibility - Updated `Cargo.toml` to include new Windows-specific dependencies. - Refactored `GrowableMemoryMap` to enhance memory allocation on Windows, including new methods for growing memory and changing permissions. - Simplified atomic operations in `PoolInner` for better clarity and performance. - Removed unused methods in `buffer.rs` to clean up the codebase. - Improved cross-platform compatibility by consolidating common implementations. - All test results give passing results --- Cargo.toml | 32 ++++++------- src/lib.rs | 26 +++++------ src/mmap.rs | 123 ++++++++++++++++++++++++++++++++++++++++--------- test/buffer.rs | 10 +--- 4 files changed, 130 insertions(+), 61 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1c48157..70aab68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,25 +1,25 @@ [package] - -name = "poule" -version = "0.3.2" -license = "MIT" -authors = ["Carl Lerche ", "Geoffroy Couprie "] -description = "A pool of reusable values" +name = "poule" +version = "0.3.2" +license = "MIT" +authors = ["Carl Lerche ", "Geoffroy Couprie "] +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" +windows-sys = { version = "0.48", features = ["Win32_Foundation", "Win32_System_Memory"] } [[test]] name = "test" diff --git a/src/lib.rs b/src/lib.rs index 640c328..6321566 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -311,15 +311,14 @@ impl PoolInner { 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); @@ -328,7 +327,7 @@ impl PoolInner { fn checkin(&self, ptr: *mut Entry) { let idx; - let mut entry: &mut Entry; + let entry: &mut Entry; unsafe { // Figure out the index @@ -344,13 +343,10 @@ impl PoolInner { // 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); } diff --git a/src/mmap.rs b/src/mmap.rs index a14bf6f..220e16c 100644 --- a/src/mmap.rs +++ b/src/mmap.rs @@ -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 @@ -26,13 +34,77 @@ pub struct GrowableMemoryMap { size: usize, } +#[cfg(windows)] +impl GrowableMemoryMap { + pub fn new(capacity: usize) -> Result { + 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 { let capacity = page_size(capacity); let ptr = unsafe { mmap( - ptr::null_mut(), + std::ptr::null_mut(), capacity, PROT_NONE, MAP_ANON | MAP_PRIVATE, @@ -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); @@ -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]; @@ -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 } } diff --git a/test/buffer.rs b/test/buffer.rs index 1f79ec7..0a005b8 100644 --- a/test/buffer.rs +++ b/test/buffer.rs @@ -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 { @@ -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()); } From 17803d8fe108b74bd5f75d6148f07ea4fc64f094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denizhan=20Dak=C4=B1l=C4=B1r?= Date: Tue, 11 Feb 2025 14:24:16 +0300 Subject: [PATCH 2/2] Update Cargo.toml --- Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 70aab68..d059fc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,8 @@ exclude = [ [dependencies] libc = "0.2" + +[target.'cfg(windows)'.dependencies] windows-sys = { version = "0.48", features = ["Win32_Foundation", "Win32_System_Memory"] } [[test]]