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

Added GlfwSender and GlfwReceiver instead of mpsc channels #552

Merged

Conversation

bohdloss
Copy link
Contributor

Original problem + my solution from a previous post:

I discovered a memory issue regarding the use of channels. Basically every kind of callback that uses polling will cause an allocation (window resizing events, mouse enter and exit events, etc...) and a subsequent deallocation. The solution i implemented was a custom channel type which i called GlfwSender / GlfwReceiver that internally uses a VecDeque in order to avoid frequent allocations. One last adjustment i made is that if the GlfwReceiver queue goes beyond a certain size, in order not to allocate too much memory for the VecDeque, it will fall back to normal mpsc channels. I initially configured the initial VecDeque capacity to be 16 and the maximum to be 256, after which mpsc channels will be used until new space is freed in the queue by flushing events. These are of course very arbitrary values and i encourage to tweak them to whatever seems suitable before merging. Let me know if this sounds like a reasonable change. Thanks!

@bohdloss
Copy link
Contributor Author

The reason i thought a change like this would make sense is that resizing, moving, keyboard and mouse events are very common and will probably be fired many times each update in a typical game/app loop scenario.

@bohdloss
Copy link
Contributor Author

To test this, i set up a test app with a custom global allocator that simply tracks the number of allocations/deallocations made before forwarding them to the default rust allocator:

#[global_allocator]
static GLOBAL: DebugAlloc = DebugAlloc;

static ALLOC_COUNT: AtomicUsize = AtomicUsize::new(0);
static DEALLOC_COUNT: AtomicUsize = AtomicUsize::new(0);

struct DebugAlloc;

unsafe impl GlobalAlloc for DebugAlloc {
	unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
		// Increment the allocation counter
		ALLOC_COUNT.fetch_add(1, Ordering::SeqCst);
		// Forward the call to the system allocator
		System::alloc(&System, layout)
	}

	unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
		// Increment the deallocation counter
		DEALLOC_COUNT.fetch_add(1, Ordering::SeqCst);
		// Forward the call to the system allocator
		System::dealloc(&System, ptr, layout)
	}
}

I enabled polling like so:

window.set_key_polling(true);
window.set_mouse_button_polling(true);
window.set_cursor_pos_polling(true);
window.set_scroll_polling(true);
window.set_size_polling(true);

The program itself is just a simple OpenGL demo with camera movement.
My tests showed multiple allocations and subsequent deallocations per second while simply moving around and rotating the camera. In debug mode i was able to track them down to the mpsc channels used by glfw-rs.
To be sure, i tested my solution that uses queues and sure enough apart from an initial allocation, no more allocations were being made.

This is a sample of the console output before the patch, taken while the demo is running (long after initialization):

[12/11/2023-3:2:13.974][GAME 1][main][Debug]: Dealloc: 1
[12/11/2023-3:2:18.183][GAME 1][main][Debug]: Alloc: 1
[12/11/2023-3:2:18.185][GAME 1][main][Debug]: Dealloc: 1
[12/11/2023-3:2:18.816][GAME 1][main][Debug]: Alloc: 1
[12/11/2023-3:2:18.817][GAME 1][main][Debug]: Dealloc: 1
[12/11/2023-3:2:19.499][GAME 1][main][Debug]: Alloc: 1
[12/11/2023-3:2:19.500][GAME 1][main][Debug]: Dealloc: 1
[12/11/2023-3:2:20.65][GAME 1][main][Debug]: Alloc: 1
[12/11/2023-3:2:20.66][GAME 1][main][Debug]: Dealloc: 1

And the console output after the patch (i included the initialization and finalization part to show that there really is no allocation inbetween):

[12/11/2023-3:15:0.797][GAME 1][*unnamed_thread*][Info]: Ready!
[12/11/2023-3:15:0.824][GAME 1][main][Debug]: Alloc: 5
[12/11/2023-3:15:0.826][GAME 1][main][Debug]: Dealloc: 1
[12/11/2023-3:15:35.109][GAME 1][*unnamed_thread*][Debug]: Sending stop signal

@bvssvni
Copy link
Member

bvssvni commented Nov 14, 2023

Merging.

@bvssvni bvssvni merged commit d51357b into PistonDevelopers:master Nov 14, 2023
@bvssvni
Copy link
Member

bvssvni commented Nov 14, 2023

Thanks!

@aloucks
Copy link
Contributor

aloucks commented Jan 25, 2024

The unbounded mpsc channel is backed by a linked list the allocates in chunks and is non-blocking. The mpsc implementation is also careful to use cache padded structures to avoid false sharing (i.e. cpu cache invalidation on other threads). You'll see allocations but they are infrequent. What problem were you trying to solve? The change here introduced a blocking channel that isn't cpu cache friendly.

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