refactor!: Remove breakwater-core crate

CHANGELOG.md

@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Changed
+
+- BREAKING: Remove the `breakwater-core` crate ([#35])
+- Add a `FrameBuffer` trait, rename the existing implementation one to `SimpleFrameBuffer` ([#35])
+
+[#35]: https://github.com/sbernauer/breakwater/pull/35
+
 ## [0.15.0] - 2024-06-12
 
 ### Added

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace]
-members = ["breakwater-core", "breakwater-parser", "breakwater"]
+members = ["breakwater-parser", "breakwater"]
 resolver = "2"
 
 [workspace.package]
@@ -14,7 +14,6 @@ chrono = "0.4"
 clap = { version = "4.5", features = ["derive"] }
 const_format = "0.2"
 criterion = {version = "0.5", features = ["async_tokio"]}
-enum_dispatch = "0.3"
 env_logger = "0.11"
 log = "0.4"
 memadvise = "0.1"

breakwater-parser/Cargo.toml

@@ -12,7 +12,7 @@ name = "parsing"
 harness = false
 
 [dependencies]
-enum_dispatch.workspace = true
+const_format.workspace = true
 memchr.workspace = true
 
 [dev-dependencies]

breakwater-parser/benches/parsing.rs

@@ -1,11 +1,9 @@
 use std::{sync::Arc, time::Duration};
 
-use breakwater_core::framebuffer::FrameBuffer;
 #[cfg(target_arch = "x86_64")]
-use breakwater_parser::assembler::AssemblerParser;
+use breakwater_parser::AssemblerParser;
 use breakwater_parser::{
-    memchr::MemchrParser, original::OriginalParser, refactored::RefactoredParser, Parser,
-    ParserImplementation,
+    MemchrParser, OriginalParser, Parser, RefactoredParser, SimpleFrameBuffer,
 };
 use criterion::{criterion_group, criterion_main, Criterion};
 use pixelbomber::image_handler::{self, ImageConfigBuilder};
@@ -99,7 +97,10 @@ fn invoke_benchmark(
 
     let mut c_group = c.benchmark_group(bench_name);
 
-    let fb = Arc::new(FrameBuffer::new(FRAMEBUFFER_WIDTH, FRAMEBUFFER_HEIGHT));
+    let fb = Arc::new(SimpleFrameBuffer::new(
+        FRAMEBUFFER_WIDTH,
+        FRAMEBUFFER_HEIGHT,
+    ));
 
     let parser_names = vec!["original", "refactored" /*"memchr"*/];
 
@@ -108,19 +109,13 @@ fn invoke_benchmark(
 
     for parse_name in parser_names {
         c_group.bench_with_input(parse_name, &commands, |b, input| {
-            b.iter(|| {
-                let mut parser = match parse_name {
-                    "original" => ParserImplementation::Original(OriginalParser::new(fb.clone())),
-                    "refactored" => {
-                        ParserImplementation::Refactored(RefactoredParser::new(fb.clone()))
-                    }
-                    "memchr" => ParserImplementation::Naive(MemchrParser::new(fb.clone())),
-                    #[cfg(target_arch = "x86_64")]
-                    "assembler" => ParserImplementation::Assembler(AssemblerParser::default()),
-                    _ => panic!("Parser implementation {parse_name} not known"),
-                };
-
-                parser.parse(input, &mut Vec::new());
+            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()),
+                #[cfg(target_arch = "x86_64")]
+                "assembler" => AssemblerParser::new(fb.clone()).parse(input, &mut Vec::new()),
+                _ => panic!("Parser implementation {parse_name} not known"),
             });
         });
     }

breakwater-parser/src/assembler.rs

@@ -4,12 +4,14 @@ use crate::{FrameBuffer, Parser};
 
 const PARSER_LOOKAHEAD: usize = "PX 1234 1234 rrggbbaa\n".len(); // Longest possible command
 
-#[derive(Default)]
-#[allow(dead_code)]
 pub struct AssemblerParser<FB: FrameBuffer> {
-    help_text: &'static [u8],
-    alt_help_text: &'static [u8],
-    fb: Arc<FB>,
+    _fb: Arc<FB>,
+}
+
+impl<FB: FrameBuffer> AssemblerParser<FB> {
+    pub fn new(_fb: Arc<FB>) -> Self {
+        Self { _fb }
+    }
 }
 
 impl<FB: FrameBuffer> Parser for AssemblerParser<FB> {

breakwater-parser/src/framebuffer/mod.rs

@@ -0,0 +1,28 @@
+pub mod simple;
+
+pub trait FrameBuffer {
+    fn get_width(&self) -> usize;
+
+    fn get_height(&self) -> usize;
+
+    fn get_size(&self) -> usize {
+        self.get_width() * self.get_height()
+    }
+
+    #[inline]
+    fn get(&self, x: usize, y: usize) -> Option<u32> {
+        if x < self.get_width() && y < self.get_height() {
+            Some(unsafe { self.get_unchecked(x, y) })
+        } else {
+            None
+        }
+    }
+
+    /// # Safety
+    /// 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 as_bytes(&self) -> &[u8];
+}

breakwater-core/src/framebuffer.rs → breakwater-parser/src/framebuffer/simple.rs

@@ -1,50 +1,41 @@
-use std::slice;
+use super::FrameBuffer;
 
-pub struct FrameBuffer {
+pub struct SimpleFrameBuffer {
     width: usize,
     height: usize,
     buffer: Vec<u32>,
 }
 
-impl FrameBuffer {
+impl SimpleFrameBuffer {
     pub fn new(width: usize, height: usize) -> Self {
         let mut buffer = Vec::with_capacity(width * height);
         buffer.resize_with(width * height, || 0);
-        FrameBuffer {
+        Self {
             width,
             height,
             buffer,
         }
     }
+}
 
-    pub fn get_width(&self) -> usize {
+impl FrameBuffer for SimpleFrameBuffer {
+    #[inline(always)]
+    fn get_width(&self) -> usize {
         self.width
     }
 
-    pub fn get_height(&self) -> usize {
-        self.height
-    }
-
-    pub fn get_size(&self) -> usize {
-        self.width * self.height
-    }
-
     #[inline(always)]
-    pub fn get(&self, x: usize, y: usize) -> Option<u32> {
-        if x < self.width && y < self.height {
-            Some(*unsafe { self.buffer.get_unchecked(x + y * self.width) })
-        } else {
-            None
-        }
+    fn get_height(&self) -> usize {
+        self.height
     }
 
     #[inline(always)]
-    pub fn get_unchecked(&self, x: usize, y: usize) -> u32 {
-        *unsafe { self.buffer.get_unchecked(x + y * self.width) }
+    unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32 {
+        *self.buffer.get_unchecked(x + y * self.width)
     }
 
     #[inline(always)]
-    pub fn set(&self, x: usize, y: usize, rgba: u32) {
+    fn set(&self, x: usize, y: usize, rgba: u32) {
         // 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
@@ -58,34 +49,9 @@ impl FrameBuffer {
         }
     }
 
-    pub fn get_buffer(&self) -> &[u32] {
-        &self.buffer
-    }
-
-    pub fn as_bytes(&self) -> &[u8] {
-        let len_in_bytes = self.buffer.len() * 4;
-        unsafe { slice::from_raw_parts(self.buffer.as_ptr() as *const u8, len_in_bytes) }
-    }
-}
-
-impl breakwater_parser::FrameBuffer for FrameBuffer {
-    #[inline]
-    fn get_width(&self) -> usize {
-        self.get_width()
-    }
-
-    #[inline]
-    fn get_height(&self) -> usize {
-        self.get_height()
-    }
-
-    #[inline]
-    unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32 {
-        self.get_unchecked(x, y)
-    }
-
-    #[inline]
-    fn set(&self, x: usize, y: usize, rgba: u32) {
-        self.set(x, y, rgba)
+    fn as_bytes(&self) -> &[u8] {
+        let len = 4 * self.buffer.len();
+        let ptr = self.buffer.as_ptr() as *const u8;
+        unsafe { std::slice::from_raw_parts(ptr, len) }
     }
 }