diff --git a/CHANGELOG.md b/CHANGELOG.md index 72dbdb3..093f517 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,25 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Added support for counting the number of pixels each IP has set and expose it as Prometheus metric + `breakwater_pixels`. Pixels are only counted when the `count-pixels` feature is explicitly + enabled, as it has a big performance impact! The feature `count-pixels` has been added as well, + which does a very crude approximation of pixels set ([#62]) + +### Changed + +- BREAKING: The Prometheus metric `breakwater_frame` has been renamed to `breakwater_vnc_frame` and + is only exported when the `vnc` feature is enabled ([#62]) + +[#62]: https://github.com/sbernauer/breakwater/pull/62 + ## [0.18.1] - 2025-05-02 ### Fixed -- Fix wrong `PXMULTI` command handling introduced in [#55] ([#60]). +- Fix wrong `PXMULTI` command handling introduced in [#55] ([#60]) [#60]: https://github.com/sbernauer/breakwater/pull/60 diff --git a/README.md b/README.md index a0b89a0..3a7e864 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,10 @@ As of writing the following features are supported: * `alpha` (disabled by default): Respect alpha values during `PX` commands. Disabled by default as this can cause performance degradation. * `binary-set-pixel` (disabled by default): Allows use of the `PB` command. * `binary-sync-pixels`(disabled by default): Allows use of the `PXMULTI` command. +* `count-pixels` (disabled by default): Count the number of pixels each IP has set and expose it as Prometheus metric `breakwater_pixels`. + Turning this feature on has a big performance impact! +* `count-pixels-approx` (disabled by default): Like `count-pixels`, but instead of actually counting the pixels, we just + divide the number of bytes by the average bytes for a pixel SET command to get a very rough estimate. To e.g. turn the VNC server off, build with diff --git a/breakwater-parser/Cargo.toml b/breakwater-parser/Cargo.toml index 5324fd5..b200bf1 100644 --- a/breakwater-parser/Cargo.toml +++ b/breakwater-parser/Cargo.toml @@ -27,5 +27,6 @@ rstest.workspace = true alpha = [] binary-set-pixel = [] binary-sync-pixels = [] +count-pixels = [] default = [] diff --git a/breakwater-parser/benches/parsing.rs b/breakwater-parser/benches/parsing.rs index 04a9f34..3b7fc78 100644 --- a/breakwater-parser/benches/parsing.rs +++ b/breakwater-parser/benches/parsing.rs @@ -110,11 +110,31 @@ fn invoke_benchmark( for parse_name in parser_names { c_group.bench_with_input(parse_name, &commands, |b, input| { b.iter(|| match parse_name { - "original" => OriginalParser::new(fb.clone()).parse(input, &mut Vec::new()), - "refactored" => RefactoredParser::new(fb.clone()).parse(input, &mut Vec::new()), - "memchr" => MemchrParser::new(fb.clone()).parse(input, &mut Vec::new()), + "original" => OriginalParser::new(fb.clone()).parse( + input, + &mut Vec::new(), + #[cfg(feature = "count-pixels")] + &breakwater_parser::NoopSetPixelsCallback, + ), + "refactored" => RefactoredParser::new(fb.clone()).parse( + input, + &mut Vec::new(), + #[cfg(feature = "count-pixels")] + &breakwater_parser::NoopSetPixelsCallback, + ), + "memchr" => MemchrParser::new(fb.clone()).parse( + input, + &mut Vec::new(), + #[cfg(feature = "count-pixels")] + &breakwater_parser::NoopSetPixelsCallback, + ), #[cfg(target_arch = "x86_64")] - "assembler" => AssemblerParser::new(fb.clone()).parse(input, &mut Vec::new()), + "assembler" => AssemblerParser::new(fb.clone()).parse( + input, + &mut Vec::new(), + #[cfg(feature = "count-pixels")] + &breakwater_parser::NoopSetPixelsCallback, + ), _ => panic!("Parser implementation {parse_name} not known"), }); }); diff --git a/breakwater-parser/src/assembler.rs b/breakwater-parser/src/assembler.rs index ed947b8..3fea010 100644 --- a/breakwater-parser/src/assembler.rs +++ b/breakwater-parser/src/assembler.rs @@ -15,7 +15,12 @@ impl AssemblerParser { } impl Parser for AssemblerParser { - fn parse(&mut self, buffer: &[u8], _response: &mut Vec) -> usize { + fn parse( + &mut self, + buffer: &[u8], + _response: &mut Vec, + #[cfg(feature = "count-pixels")] _set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize { let mut last_byte_parsed = 0; // This loop does nothing and should be seen as a placeholder diff --git a/breakwater-parser/src/framebuffer/mod.rs b/breakwater-parser/src/framebuffer/mod.rs index bff496f..d8b4098 100644 --- a/breakwater-parser/src/framebuffer/mod.rs +++ b/breakwater-parser/src/framebuffer/mod.rs @@ -31,7 +31,13 @@ pub trait FrameBuffer { /// make sure x and y are in bounds unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32; - fn set(&self, x: usize, y: usize, rgba: u32); + fn set( + &self, + x: usize, + y: usize, + rgba: u32, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ); /// We can *not* take an `&[u32]` for the pixel here, as `std::slice::from_raw_parts` requires the data to be /// aligned. As the data already is stored in a buffer we can not guarantee it's correctly aligned, so let's just @@ -39,9 +45,20 @@ pub trait FrameBuffer { /// /// Returns the coordinates where we landed after filling #[inline(always)] - fn set_multi(&self, start_x: usize, start_y: usize, pixels: &[u8]) -> (usize, usize) { + fn set_multi( + &self, + start_x: usize, + start_y: usize, + pixels: &[u8], + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> (usize, usize) { let starting_index = start_x + start_y * self.get_width(); - let pixels_copied = self.set_multi_from_start_index(starting_index, pixels); + let pixels_copied = self.set_multi_from_start_index( + starting_index, + pixels, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); let new_x = (start_x + pixels_copied) % self.get_width(); let new_y = start_y + (pixels_copied / self.get_width()); @@ -50,7 +67,12 @@ pub trait FrameBuffer { } /// Returns the number of pixels copied - fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize; + fn set_multi_from_start_index( + &self, + starting_index: usize, + pixels: &[u8], + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize; /// As the pixel memory doesn't necessarily need to be aligned (think of using shared memory for /// that), we can only return it as a list of bytes, not a list of pixels. diff --git a/breakwater-parser/src/framebuffer/shared_memory.rs b/breakwater-parser/src/framebuffer/shared_memory.rs index 0ce0665..d46a2b9 100644 --- a/breakwater-parser/src/framebuffer/shared_memory.rs +++ b/breakwater-parser/src/framebuffer/shared_memory.rs @@ -175,7 +175,13 @@ impl FrameBuffer for SharedMemoryFrameBuffer { } #[inline(always)] - fn set(&self, x: usize, y: usize, rgba: u32) { + fn set( + &self, + x: usize, + y: usize, + rgba: u32, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) { // See 'SimpleFrameBuffer::set' for performance consideration if x < self.width && y < self.height { let offset = (x + y * self.width) * FB_BYTES_PER_PIXEL; @@ -183,11 +189,19 @@ impl FrameBuffer for SharedMemoryFrameBuffer { // The buffer coming from the shared memory might be unaligned! unsafe { pixel_ptr.write_unaligned(rgba) } + + #[cfg(feature = "count-pixels")] + set_pixels_callback.pixels_set(1); } } #[inline(always)] - fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize { + fn set_multi_from_start_index( + &self, + starting_index: usize, + pixels: &[u8], + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize { let num_pixels = pixels.len() / FB_BYTES_PER_PIXEL; if starting_index + num_pixels > self.get_size() { @@ -209,6 +223,13 @@ impl FrameBuffer for SharedMemoryFrameBuffer { let target_slice = unsafe { slice::from_raw_parts_mut(starting_ptr, pixels.len()) }; target_slice.copy_from_slice(pixels); + #[cfg(feature = "count-pixels")] + set_pixels_callback.pixels_set( + num_pixels + .try_into() + .expect("More than u64::MAX pixels colored!"), + ); + num_pixels } diff --git a/breakwater-parser/src/framebuffer/simple.rs b/breakwater-parser/src/framebuffer/simple.rs index 0a9a68c..d602fa4 100644 --- a/breakwater-parser/src/framebuffer/simple.rs +++ b/breakwater-parser/src/framebuffer/simple.rs @@ -39,7 +39,13 @@ impl FrameBuffer for SimpleFrameBuffer { } #[inline(always)] - fn set(&self, x: usize, y: usize, rgba: u32) { + fn set( + &self, + x: usize, + y: usize, + rgba: u32, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) { // https://github.com/sbernauer/breakwater/pull/11 // If we make the FrameBuffer large enough (e.g. 10_000 x 10_000) we don't need to check the bounds here // (x and y are max 4 digit numbers). Flamegraph has shown 5.21% of runtime in this bound check. On the other @@ -50,11 +56,19 @@ impl FrameBuffer for SimpleFrameBuffer { let ptr = self.buffer.as_ptr().add(x + y * self.width) as *mut u32; *ptr = rgba; } + + #[cfg(feature = "count-pixels")] + set_pixels_callback.pixels_set(1); } } #[inline(always)] - fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize { + fn set_multi_from_start_index( + &self, + starting_index: usize, + pixels: &[u8], + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize { let num_pixels = pixels.len() / FB_BYTES_PER_PIXEL; if starting_index + num_pixels > self.buffer.len() { @@ -73,6 +87,13 @@ impl FrameBuffer for SimpleFrameBuffer { unsafe { slice::from_raw_parts_mut(starting_ptr as *mut u8, pixels.len()) }; target_slice.copy_from_slice(pixels); + #[cfg(feature = "count-pixels")] + set_pixels_callback.pixels_set( + num_pixels + .try_into() + .expect("More than u64::MAX pixels colored!"), + ); + num_pixels } @@ -107,7 +128,13 @@ mod tests { #[case] y: usize, #[case] rgba: u32, ) { - fb.set(x, y, rgba); + fb.set( + x, + y, + rgba, + #[cfg(feature = "count-pixels")] + &crate::NoopSetPixelsCallback, + ); assert_eq!(fb.get(x, y), Some(rgba)); } @@ -122,7 +149,13 @@ mod tests { let pixels = (0..10_u32).collect::>(); let pixel_bytes: Vec = pixels.iter().flat_map(|p| p.to_le_bytes()).collect(); - let (current_x, current_y) = fb.set_multi(0, 0, &pixel_bytes); + let (current_x, current_y) = fb.set_multi( + 0, + 0, + &pixel_bytes, + #[cfg(feature = "count-pixels")] + &crate::NoopSetPixelsCallback, + ); assert_eq!(current_x, 10); assert_eq!(current_y, 0); @@ -143,7 +176,13 @@ mod tests { // Let's color exactly 3 lines and 42 pixels let pixels = (0..3 * fb.width as u32 + 42).collect::>(); let pixel_bytes: Vec = pixels.iter().flat_map(|p| p.to_le_bytes()).collect(); - let (current_x, current_y) = fb.set_multi(x, y, &pixel_bytes); + let (current_x, current_y) = fb.set_multi( + x, + y, + &pixel_bytes, + #[cfg(feature = "count-pixels")] + &crate::NoopSetPixelsCallback, + ); assert_eq!(current_x, 52); assert_eq!(current_y, 103); @@ -175,7 +214,13 @@ mod tests { pub fn test_set_multi_does_nothing_when_too_long(fb: SimpleFrameBuffer) { let mut too_long = Vec::with_capacity(fb.width * fb.height * FB_BYTES_PER_PIXEL); too_long.fill_with(|| 42_u8); - let (current_x, current_y) = fb.set_multi(1, 0, &too_long); + let (current_x, current_y) = fb.set_multi( + 1, + 0, + &too_long, + #[cfg(feature = "count-pixels")] + &crate::NoopSetPixelsCallback, + ); // Should be unchanged assert_eq!(current_x, 1); diff --git a/breakwater-parser/src/lib.rs b/breakwater-parser/src/lib.rs index dceadd5..bc0d069 100644 --- a/breakwater-parser/src/lib.rs +++ b/breakwater-parser/src/lib.rs @@ -52,8 +52,30 @@ pub const ALT_HELP_TEXT: &[u8] = b"Stop spamming HELP!\n"; pub trait Parser { /// Returns the last byte parsed. The next parsing loop will again contain all data that was not parsed. - fn parse(&mut self, buffer: &[u8], response: &mut Vec) -> usize; + fn parse( + &mut self, + buffer: &[u8], + response: &mut Vec, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl SetPixelsCallback, + ) -> usize; // Sadly this cant be const (yet?) (https://github.com/rust-lang/rust/issues/71971 and https://github.com/rust-lang/rfcs/pull/2632) fn parser_lookahead(&self) -> usize; } +#[cfg(feature = "count-pixels")] +pub trait SetPixelsCallback { + /// This function is called for every pixel or set of pixels that are set. + /// + /// It get's one parameter `pixels`: The number of pixels set. + /// + /// For performance reasons it's neither `async` nor fallible. + fn pixels_set(&self, pixels: u64); +} + +#[cfg(feature = "count-pixels")] +pub struct NoopSetPixelsCallback; + +#[cfg(feature = "count-pixels")] +impl SetPixelsCallback for NoopSetPixelsCallback { + fn pixels_set(&self, _pixels: u64) {} +} diff --git a/breakwater-parser/src/memchr.rs b/breakwater-parser/src/memchr.rs index d91327b..d219a33 100644 --- a/breakwater-parser/src/memchr.rs +++ b/breakwater-parser/src/memchr.rs @@ -13,7 +13,12 @@ impl MemchrParser { } impl Parser for MemchrParser { - fn parse(&mut self, buffer: &[u8], _response: &mut Vec) -> usize { + fn parse( + &mut self, + buffer: &[u8], + _response: &mut Vec, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize { let mut last_char_after_newline = 0; for newline in memchr::memchr_iter(b'\n', buffer) { // TODO Use get_unchecked everywhere @@ -53,7 +58,13 @@ impl Parser for MemchrParser { .parse() .expect("rgba was not a number"); - self.fb.set(x as usize, y as usize, rgba); + self.fb.set( + x as usize, + y as usize, + rgba, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } _ => { continue; diff --git a/breakwater-parser/src/original.rs b/breakwater-parser/src/original.rs index a5ff9a6..5deddec 100644 --- a/breakwater-parser/src/original.rs +++ b/breakwater-parser/src/original.rs @@ -45,7 +45,12 @@ impl OriginalParser { } impl Parser for OriginalParser { - fn parse(&mut self, buffer: &[u8], response: &mut Vec) -> usize { + fn parse( + &mut self, + buffer: &[u8], + response: &mut Vec, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize { let mut last_byte_parsed = 0; let mut help_count = 0; @@ -58,10 +63,12 @@ impl Parser for OriginalParser { if remaining.bytes_remaining <= buffer.len() { // Easy going here - self.fb - .set_multi_from_start_index(remaining.current_index, unsafe { - slice::from_raw_parts(buffer.as_ptr(), remaining.bytes_remaining) - }); + self.fb.set_multi_from_start_index( + remaining.current_index, + unsafe { slice::from_raw_parts(buffer.as_ptr(), remaining.bytes_remaining) }, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); i += remaining.bytes_remaining; last_byte_parsed = i; self.remaining_pixel_sync = None; @@ -73,11 +80,12 @@ impl Parser for OriginalParser { let pixel_bytes = buffer.len() / 4 * 4; let mut index = remaining.current_index; - index += self - .fb - .set_multi_from_start_index(remaining.current_index, unsafe { - slice::from_raw_parts(buffer.as_ptr(), pixel_bytes) - }); + index += self.fb.set_multi_from_start_index( + remaining.current_index, + unsafe { slice::from_raw_parts(buffer.as_ptr(), pixel_bytes) }, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); self.remaining_pixel_sync = Some(RemainingPixelSync { current_index: index, @@ -117,7 +125,13 @@ impl Parser for OriginalParser { let rgba: u32 = simd_unhex(unsafe { buffer.as_ptr().add(i - 7) }); - self.fb.set(x, y, rgba & 0x00ff_ffff); + self.fb.set( + x, + y, + rgba & 0x00ff_ffff, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); continue; } @@ -129,7 +143,14 @@ impl Parser for OriginalParser { let rgba: u32 = simd_unhex(unsafe { buffer.as_ptr().add(i - 9) }); - self.fb.set(x, y, rgba & 0x00ff_ffff); + self.fb.set( + x, + y, + rgba & 0x00ff_ffff, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); + continue; } #[cfg(feature = "alpha")] @@ -155,7 +176,14 @@ impl Parser for OriginalParser { let g: u32 = (((current >> 16) & 0xff) * alpha_comp + g * alpha) / 0xff; let b: u32 = (((current >> 8) & 0xff) * alpha_comp + b * alpha) / 0xff; - self.fb.set(x, y, (r << 16) | (g << 8) | b); + self.fb.set( + x, + y, + (r << 16) | (g << 8) | b, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); + continue; } @@ -168,7 +196,13 @@ impl Parser for OriginalParser { let rgba: u32 = (base << 16) | (base << 8) | base; - self.fb.set(x, y, rgba); + self.fb.set( + x, + y, + rgba, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); continue; } @@ -204,7 +238,13 @@ impl Parser for OriginalParser { let rgba = u32::from_le((command_bytes >> 32) as u32); // TODO: Support alpha channel (behind alpha feature flag) - self.fb.set(x as usize, y as usize, rgba & 0x00ff_ffff); + self.fb.set( + x as usize, + y as usize, + rgba & 0x00ff_ffff, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); // P B XX YY RGBA last_byte_parsed = i + 1 + 2 + 2 + 4; i += 10; @@ -224,10 +264,13 @@ impl Parser for OriginalParser { if len_in_bytes <= bytes_left_in_buffer { // Easy going here - self.fb - .set_multi(start_x as usize, start_y as usize, unsafe { - slice::from_raw_parts(buffer.as_ptr().add(i), len_in_bytes) - }); + self.fb.set_multi( + start_x as usize, + start_y as usize, + unsafe { slice::from_raw_parts(buffer.as_ptr().add(i), len_in_bytes) }, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); i += len_in_bytes; last_byte_parsed = i; @@ -240,9 +283,12 @@ impl Parser for OriginalParser { // what the client is doing. let mut current_index = start_x as usize + start_y as usize * self.fb.get_width(); - current_index += self.fb.set_multi_from_start_index(current_index, unsafe { - slice::from_raw_parts(buffer.as_ptr().add(i), pixel_bytes) - }); + current_index += self.fb.set_multi_from_start_index( + current_index, + unsafe { slice::from_raw_parts(buffer.as_ptr().add(i), pixel_bytes) }, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); self.remaining_pixel_sync = Some(RemainingPixelSync { current_index, diff --git a/breakwater-parser/src/refactored.rs b/breakwater-parser/src/refactored.rs index a8c901c..4797614 100644 --- a/breakwater-parser/src/refactored.rs +++ b/breakwater-parser/src/refactored.rs @@ -31,6 +31,7 @@ impl RefactoredParser { buffer: &[u8], mut idx: usize, response: &mut Vec, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, ) -> (usize, usize) { let previous = idx; idx += 3; @@ -51,19 +52,40 @@ impl RefactoredParser { // Must be followed by 6 bytes RGB and newline or ... if unsafe { *buffer.get_unchecked(idx + 6) } == b'\n' { idx += 7; - self.handle_rgb(idx, buffer, x, y); + self.handle_rgb( + idx, + buffer, + x, + y, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); (idx, idx) } // ... or must be followed by 8 bytes RGBA and newline else if unsafe { *buffer.get_unchecked(idx + 8) } == b'\n' { idx += 9; - self.handle_rgba(idx, buffer, x, y); + self.handle_rgba( + idx, + buffer, + x, + y, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); (idx, idx) } // ... for the efficient/lazy clients else if unsafe { *buffer.get_unchecked(idx + 2) } == b'\n' { idx += 3; - self.handle_gray(idx, buffer, x, y); + self.handle_gray( + idx, + buffer, + x, + y, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); (idx, idx) } else { (idx, previous) @@ -83,7 +105,12 @@ impl RefactoredParser { } #[inline(always)] - fn handle_binary_pixel(&self, buffer: &[u8], mut idx: usize) -> (usize, usize) { + fn handle_binary_pixel( + &self, + buffer: &[u8], + mut idx: usize, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> (usize, usize) { let previous = idx; idx += 2; @@ -94,7 +121,13 @@ impl RefactoredParser { let rgba = u32::from_le((command_bytes >> 32) as u32); // TODO: Support alpha channel (behind alpha feature flag) - self.fb.set(x as usize, y as usize, rgba & 0x00ff_ffff); + self.fb.set( + x as usize, + y as usize, + rgba & 0x00ff_ffff, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); idx += 8; (idx, previous) @@ -124,23 +157,56 @@ impl RefactoredParser { } #[inline(always)] - fn handle_rgb(&self, idx: usize, buffer: &[u8], x: usize, y: usize) { + fn handle_rgb( + &self, + idx: usize, + buffer: &[u8], + x: usize, + y: usize, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) { let rgba: u32 = simd_unhex(unsafe { buffer.as_ptr().add(idx - 7) }); - self.fb.set(x, y, rgba & 0x00ff_ffff); + self.fb.set( + x, + y, + rgba & 0x00ff_ffff, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } #[cfg(not(feature = "alpha"))] #[inline(always)] - fn handle_rgba(&self, idx: usize, buffer: &[u8], x: usize, y: usize) { + fn handle_rgba( + &self, + idx: usize, + buffer: &[u8], + x: usize, + y: usize, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) { let rgba: u32 = simd_unhex(unsafe { buffer.as_ptr().add(idx - 9) }); - self.fb.set(x, y, rgba & 0x00ff_ffff); + self.fb.set( + x, + y, + rgba & 0x00ff_ffff, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } #[cfg(feature = "alpha")] #[inline(always)] - fn handle_rgba(&self, idx: usize, buffer: &[u8], x: usize, y: usize) { + fn handle_rgba( + &self, + idx: usize, + buffer: &[u8], + x: usize, + y: usize, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) { let rgba: u32 = simd_unhex(unsafe { buffer.as_ptr().add(idx - 9) }); let alpha = (rgba >> 24) & 0xff; @@ -159,18 +225,37 @@ impl RefactoredParser { let g: u32 = (((current >> 16) & 0xff) * alpha_comp + g * alpha) / 0xff; let b: u32 = (((current >> 8) & 0xff) * alpha_comp + b * alpha) / 0xff; - self.fb.set(x, y, (r << 16) | (g << 8) | b); + self.fb.set( + x, + y, + (r << 16) | (g << 8) | b, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } #[inline(always)] - fn handle_gray(&self, idx: usize, buffer: &[u8], x: usize, y: usize) { + fn handle_gray( + &self, + idx: usize, + buffer: &[u8], + x: usize, + y: usize, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) { // FIXME: Read that two bytes directly instead of going through the whole SIMD vector setup. // Or - as an alternative - still do the SIMD part but only load two bytes. let base: u32 = simd_unhex(unsafe { buffer.as_ptr().add(idx - 3) }) & 0xff; let rgba: u32 = (base << 16) | (base << 8) | base; - self.fb.set(x, y, rgba); + self.fb.set( + x, + y, + rgba, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } #[inline(always)] @@ -191,7 +276,12 @@ impl RefactoredParser { } impl Parser for RefactoredParser { - fn parse(&mut self, buffer: &[u8], response: &mut Vec) -> usize { + fn parse( + &mut self, + buffer: &[u8], + response: &mut Vec, + #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback, + ) -> usize { let mut last_byte_parsed = 0; let mut i = 0; // We can't use a for loop here because Rust don't lets use skip characters by incrementing i @@ -201,11 +291,22 @@ impl Parser for RefactoredParser { let current_command = unsafe { (buffer.as_ptr().add(i) as *const u64).read_unaligned() }; if current_command & 0x00ff_ffff == PX_PATTERN { - (i, last_byte_parsed) = self.handle_pixel(buffer, i, response); + (i, last_byte_parsed) = self.handle_pixel( + buffer, + i, + response, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } else if cfg!(feature = "binary-set-pixel") && current_command & 0x0000_ffff == PB_PATTERN { - (i, last_byte_parsed) = self.handle_binary_pixel(buffer, i); + (i, last_byte_parsed) = self.handle_binary_pixel( + buffer, + i, + #[cfg(feature = "count-pixels")] + set_pixels_callback, + ); } else if current_command & 0x00ff_ffff_ffff_ffff == OFFSET_PATTERN { i += 7; self.handle_offset(&mut i, buffer); diff --git a/breakwater/Cargo.toml b/breakwater/Cargo.toml index 0afad82..500b12c 100644 --- a/breakwater/Cargo.toml +++ b/breakwater/Cargo.toml @@ -51,6 +51,8 @@ default = ["egui", "vnc"] alpha = ["breakwater-parser/alpha"] binary-set-pixel = ["breakwater-parser/binary-set-pixel"] binary-sync-pixels = ["breakwater-parser/binary-sync-pixels"] +count-pixels = ["breakwater-parser/count-pixels"] +count-pixels-approx = [] egui = ["dep:breakwater-egui-overlay", "dep:bytemuck", "dep:eframe", "dep:egui", "dep:libloading"] native-display = ["dep:softbuffer", "dep:winit"] vnc = ["dep:vncserver"] diff --git a/breakwater/src/main.rs b/breakwater/src/main.rs index 1bf5ddd..b9d0456 100644 --- a/breakwater/src/main.rs +++ b/breakwater/src/main.rs @@ -48,7 +48,13 @@ async fn main() -> eyre::Result<()> { // If we make the channel to big, stats will start to lag behind // TODO: Check performance impact in real-world scenario. Maybe the statistics thread blocks the other threads - let (statistics_tx, statistics_rx) = mpsc::channel::(100); + let (statistics_tx, statistics_rx) = + mpsc::channel::(if cfg!(feature = "count-pixels") { + // Pixel counting is currently done via StatisticEvents, resulting in high numbers! + 10000 + } else { + 100 + }); let (statistics_information_tx, statistics_information_rx) = broadcast::channel::(2); let (terminate_signal_tx, terminate_signal_rx) = broadcast::channel::<()>(1); diff --git a/breakwater/src/prometheus_exporter.rs b/breakwater/src/prometheus_exporter.rs index 1572e75..deadd4d 100644 --- a/breakwater/src/prometheus_exporter.rs +++ b/breakwater/src/prometheus_exporter.rs @@ -13,12 +13,17 @@ pub struct PrometheusExporter { // Prometheus metrics metric_ips_v6: IntGauge, metric_ips_v4: IntGauge, - metric_frame: IntGauge, metric_statistic_events: IntGauge, metric_connections_for_ip: IntGaugeVec, metric_denied_connections_for_ip: IntGaugeVec, metric_bytes_for_ip: IntGaugeVec, + + #[cfg(feature = "vnc")] + metric_vnc_frame: IntGauge, + + #[cfg(any(feature = "count-pixels", feature = "count-pixels-approx"))] + metric_pixels_for_ip: IntGaugeVec, } impl PrometheusExporter { @@ -39,10 +44,6 @@ impl PrometheusExporter { "breakwater_ips_v4", "Total number of connected IPv4 addresses", )?, - metric_frame: register_int_gauge!( - "breakwater_frame", - "Frame number of the VNC server" - )?, metric_statistic_events: register_int_gauge!( "breakwater_statistic_events", "Number of statistics events send internally", @@ -62,6 +63,21 @@ impl PrometheusExporter { "Number of bytes received per IP address", &["ip"], )?, + #[cfg(feature = "vnc")] + metric_vnc_frame: register_int_gauge!( + "breakwater_vnc_frame", + "Frame number of the VNC server" + )?, + #[cfg(any(feature = "count-pixels", feature = "count-pixels-approx"))] + metric_pixels_for_ip: register_int_gauge_vec!( + "breakwater_pixels", + if cfg!(feature = "count-pixels") { + "Number of pixels colored per IP address" + } else { + "Number of pixels colored per IP address (approximated from the number oy bytes send)" + }, + &["ip"], + )?, }) } @@ -69,7 +85,6 @@ impl PrometheusExporter { while let Ok(event) = self.statistics_information_rx.recv().await { self.metric_ips_v6.set(event.ips_v6 as i64); self.metric_ips_v4.set(event.ips_v4 as i64); - self.metric_frame.set(event.frame as i64); self.metric_statistic_events .set(event.statistic_events as i64); @@ -99,6 +114,34 @@ impl PrometheusExporter { .with_label_values(&[&ip.to_string()]) .set(*bytes as i64) }); + + #[cfg(feature = "vnc")] + self.metric_vnc_frame.set(event.vnc_frame as i64); + + #[cfg(feature = "count-pixels")] + { + self.metric_pixels_for_ip.reset(); + event.pixels_for_ip.iter().for_each(|(ip, pixels)| { + self.metric_pixels_for_ip + .with_label_values(&[&ip.to_string()]) + .set(*pixels as i64) + }); + } + + #[cfg(all(feature = "count-pixels-approx", not(feature = "count-pixels")))] + { + // TODO: We could do some better approximation. Maybe as const calculation? :) + // Or measure at a real event. + #[cfg(feature = "count-pixels-approx")] + const AVG_BYTES_PER_SET_COMMAND: f64 = "PX 123 123 rrggbb".len() as f64; + + self.metric_pixels_for_ip.reset(); + event.bytes_for_ip.iter().for_each(|(ip, bytes)| { + self.metric_pixels_for_ip + .with_label_values(&[&ip.to_string()]) + .set((*bytes as f64 / AVG_BYTES_PER_SET_COMMAND) as i64) + }); + } } } } diff --git a/breakwater/src/server.rs b/breakwater/src/server.rs index 26f90c1..d7dbd91 100644 --- a/breakwater/src/server.rs +++ b/breakwater/src/server.rs @@ -142,6 +142,10 @@ pub async fn handle_connection( ) -> eyre::Result<()> { tracing::debug!("handling new connection"); + #[cfg(feature = "count-pixels")] + let set_pixels_callback = + crate::statistics::StatisticsSetPixelsCallback::new(statistics_tx.clone(), ip); + statistics_tx .send(StatisticsEvent::ConnectionCreated { ip }) .await @@ -209,8 +213,12 @@ pub async fn handle_connection( *i = 0; } - let last_byte_parsed = - parser.parse(&buffer[..data_end + parser_lookahead], &mut response_buf); + let last_byte_parsed = parser.parse( + &buffer[..data_end + parser_lookahead], + &mut response_buf, + #[cfg(feature = "count-pixels")] + &set_pixels_callback, + ); if !response_buf.is_empty() { stream diff --git a/breakwater/src/statistics.rs b/breakwater/src/statistics.rs index 3e2e7c8..0a8c543 100644 --- a/breakwater/src/statistics.rs +++ b/breakwater/src/statistics.rs @@ -36,6 +36,11 @@ pub enum StatisticsEvent { ip: IpAddr, bytes: u64, }, + #[cfg(feature = "count-pixels")] + PixelSet { + ip: IpAddr, + pixels: u64, + }, #[cfg(feature = "vnc")] VncFrameRendered, } @@ -47,18 +52,22 @@ pub enum StatisticsSaveMode { #[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct StatisticsInformationEvent { - pub frame: u64, pub connections: u32, pub ips_v6: u32, pub ips_v4: u32, pub bytes: u64, - pub fps: u64, pub bytes_per_s: u64, pub connections_for_ip: HashMap, pub denied_connections_for_ip: HashMap, pub bytes_for_ip: HashMap, + #[cfg(feature = "vnc")] + pub vnc_frame: u64, + + #[cfg(feature = "count-pixels")] + pub pixels_for_ip: HashMap, + pub statistic_events: u64, } @@ -67,13 +76,16 @@ pub struct Statistics { statistics_information_tx: broadcast::Sender, statistic_events: u64, - frame: u64, connections_for_ip: HashMap, denied_connections_for_ip: HashMap, bytes_for_ip: HashMap, - bytes_per_s_window: SingleSumSMA, - fps_window: SingleSumSMA, + + #[cfg(feature = "vnc")] + vnc_frame: u64, + + #[cfg(feature = "count-pixels")] + pixels_for_ip: HashMap, statistics_save_mode: StatisticsSaveMode, } @@ -108,12 +120,14 @@ impl Statistics { statistics_rx, statistics_information_tx, statistic_events: 0, - frame: 0, connections_for_ip: HashMap::new(), denied_connections_for_ip: HashMap::new(), bytes_for_ip: HashMap::new(), bytes_per_s_window: SingleSumSMA::new(), - fps_window: SingleSumSMA::new(), + #[cfg(feature = "count-pixels")] + pixels_for_ip: HashMap::new(), + #[cfg(feature = "vnc")] + vnc_frame: 0, statistics_save_mode, }; @@ -121,7 +135,10 @@ impl Statistics { // There might not be a save point on first start if let Ok(save_point) = StatisticsInformationEvent::load_from_file(save_file) { statistics.statistic_events = save_point.statistic_events; - statistics.frame = save_point.frame; + #[cfg(feature = "vnc")] + { + statistics.vnc_frame = save_point.vnc_frame; + } statistics.bytes_for_ip = save_point.bytes_for_ip; } } @@ -198,8 +215,12 @@ impl Statistics { StatisticsEvent::BytesRead { ip, bytes } => { *self.bytes_for_ip.entry(ip).or_insert(0) += bytes; } + #[cfg(feature = "count-pixels")] + StatisticsEvent::PixelSet { ip, pixels: count } => { + *self.pixels_for_ip.entry(ip).or_insert(0) += count; + } #[cfg(feature = "vnc")] - StatisticsEvent::VncFrameRendered => self.frame += 1, + StatisticsEvent::VncFrameRendered => self.vnc_frame += 1, } } @@ -209,7 +230,8 @@ impl Statistics { elapsed: Duration, ) -> StatisticsInformationEvent { let elapsed_ms = max(1, elapsed.as_millis()) as u64; - let frame = self.frame; + #[cfg(feature = "vnc")] + let vnc_frame = self.vnc_frame; let connections = self.connections_for_ip.values().sum(); let [ips_v6, ips_v4] = self .connections_for_ip @@ -221,22 +243,65 @@ impl Statistics { let bytes = self.bytes_for_ip.values().sum(); self.bytes_per_s_window .add_sample((bytes - prev.bytes) * 1000 / elapsed_ms); - self.fps_window - .add_sample((frame - prev.frame) * 1000 / elapsed_ms); let statistic_events = self.statistic_events; StatisticsInformationEvent { - frame, connections, ips_v6, ips_v4, bytes, - fps: self.fps_window.get_average(), bytes_per_s: self.bytes_per_s_window.get_average(), connections_for_ip: self.connections_for_ip.clone(), denied_connections_for_ip: self.denied_connections_for_ip.clone(), bytes_for_ip: self.bytes_for_ip.clone(), + #[cfg(feature = "vnc")] + vnc_frame, + #[cfg(feature = "count-pixels")] + pixels_for_ip: self.pixels_for_ip.clone(), statistic_events, } } } + +#[cfg(feature = "count-pixels")] +pub struct StatisticsSetPixelsCallback { + statistics_tx: mpsc::Sender, + ip: IpAddr, +} + +#[cfg(feature = "count-pixels")] +impl StatisticsSetPixelsCallback { + pub fn new(statistics_tx: mpsc::Sender, ip: IpAddr) -> Self { + Self { statistics_tx, ip } + } +} + +#[cfg(feature = "count-pixels")] +impl breakwater_parser::SetPixelsCallback for StatisticsSetPixelsCallback { + fn pixels_set(&self, pixels: u64) { + let tx = self.statistics_tx.clone(); + let ip = self.ip; + + // Yes I know, this is slow, I would love to see a more elegant solution. + // + // I don't want to use `try_send` (a synchronous function) here, as statistics events should + // not be dropped. + // + // `blocking_send` results in + // Cannot block the current thread from within a runtime. This happens because a function attempted to block the + // current thread while the thread is being used to drive asynchronous tasks. + // + // We could use `std::sync::mpsc`, but I don't want to sacrifice performance + // for non-pixel-counting scenarios, as they are more performance sensitive. + // + // I guess we could use a different `mpsc` implementation (tokio/std) based on the + // "count-pixels" feature, but that sounds a bit overkill to me for now. + tokio::spawn(async move { + if let Err(err) = tx.send(StatisticsEvent::PixelSet { ip, pixels }).await { + tracing::warn!( + ip = %ip, pixels, err = &err as &dyn std::error::Error, "Failed to send statistics event for PixelSet" + ); + } + }); + } +}