From abbe85b7e5839a842a4e4d6492ab64cc2f4e6434 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 25 Nov 2021 05:06:10 +0100 Subject: [PATCH 1/7] fix #628 --- firmware/defmt-rtt/Cargo.toml | 1 + firmware/defmt-rtt/README.md | 6 ++++++ firmware/defmt-rtt/src/lib.rs | 16 +++++++++++++--- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/firmware/defmt-rtt/Cargo.toml b/firmware/defmt-rtt/Cargo.toml index bc25aa2f..16c67799 100644 --- a/firmware/defmt-rtt/Cargo.toml +++ b/firmware/defmt-rtt/Cargo.toml @@ -12,4 +12,5 @@ version = "0.3.0" [dependencies] cortex-m = "0.7" +konst = "0.2" defmt = { version = "0.3", path = "../../defmt" } diff --git a/firmware/defmt-rtt/README.md b/firmware/defmt-rtt/README.md index 7c69179c..630313ae 100644 --- a/firmware/defmt-rtt/README.md +++ b/firmware/defmt-rtt/README.md @@ -12,6 +12,12 @@ The fastest way to get started with `defmt` is to use our [app-template] to set For more details about the framework check the book at https://defmt.ferrous-systems.com +## Customization + +The RTT buffer size (default: 1024) can be configured with the `DEFMT_RTT_BUFFER_SIZE` environment variable. +- use a power of 2 for best performance +- caution: the `sccache` compiler cache is not supported because [it does not honor environment variables](https://github.com/mozilla/sccache/blob/master/docs/Rust.md). + ## Support `defmt-rtt` is part of the [Knurling] project, [Ferrous Systems]' effort at diff --git a/firmware/defmt-rtt/src/lib.rs b/firmware/defmt-rtt/src/lib.rs index bdc68778..97f755db 100644 --- a/firmware/defmt-rtt/src/lib.rs +++ b/firmware/defmt-rtt/src/lib.rs @@ -94,9 +94,19 @@ const MODE_BLOCK_IF_FULL: usize = 2; /// Don't block if the RTT buffer is full. Truncate data to output as much as fits. const MODE_NON_BLOCKING_TRIM: usize = 1; -// TODO make configurable -// NOTE use a power of 2 for best performance -const SIZE: usize = 1024; +// buffer size. Default: 1024; can be customized by setting `DEFMT_RTT_BUFFER_SIZE` +// NOTE +// - use a power of 2 for best performance +// - `sccache` does not honor environment variables, so unset `RUSTC_WRAPPER` when customizing. +const SIZE: usize = size(); +const fn size() -> usize { + if let Some(size_s) = option_env!("DEFMT_RTT_BUFFER_SIZE") { + if let Ok(size) = konst::primitive::parse_usize(size_s) { + return size; + } + }; + 1024 +} // make sure we only get shared references to the header/channel (avoid UB) /// # Safety From a05f3347a041b89e92a6e24041fa93d9415e3519 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 25 Nov 2021 12:05:10 +0100 Subject: [PATCH 2/7] dox++ --- firmware/defmt-rtt/src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/firmware/defmt-rtt/src/lib.rs b/firmware/defmt-rtt/src/lib.rs index 97f755db..d4cb66de 100644 --- a/firmware/defmt-rtt/src/lib.rs +++ b/firmware/defmt-rtt/src/lib.rs @@ -94,10 +94,9 @@ const MODE_BLOCK_IF_FULL: usize = 2; /// Don't block if the RTT buffer is full. Truncate data to output as much as fits. const MODE_NON_BLOCKING_TRIM: usize = 1; -// buffer size. Default: 1024; can be customized by setting `DEFMT_RTT_BUFFER_SIZE` -// NOTE -// - use a power of 2 for best performance -// - `sccache` does not honor environment variables, so unset `RUSTC_WRAPPER` when customizing. +/// buffer size. Default: 1024; can be customized by setting `DEFMT_RTT_BUFFER_SIZE` +/// - use a power of 2 for best performance +/// - `sccache` does not honor environment variables, so unset `RUSTC_WRAPPER` when customizing. const SIZE: usize = size(); const fn size() -> usize { if let Some(size_s) = option_env!("DEFMT_RTT_BUFFER_SIZE") { From 0ca1b1470f583b2dc23b6cea042d9c0c247b03a6 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 25 Nov 2021 12:05:10 +0100 Subject: [PATCH 3/7] - switch to `build.rs` buffer size evaluation to work around `sccache` limitations - add check for NPOT --- firmware/defmt-rtt/README.md | 4 +--- firmware/defmt-rtt/build.rs | 23 +++++++++++++++++++++++ firmware/defmt-rtt/src/channel.rs | 22 +++++++++++----------- firmware/defmt-rtt/src/consts.rs | 2 ++ firmware/defmt-rtt/src/lib.rs | 24 ++++++++---------------- 5 files changed, 45 insertions(+), 30 deletions(-) create mode 100644 firmware/defmt-rtt/build.rs create mode 100644 firmware/defmt-rtt/src/consts.rs diff --git a/firmware/defmt-rtt/README.md b/firmware/defmt-rtt/README.md index 630313ae..72cbe0c9 100644 --- a/firmware/defmt-rtt/README.md +++ b/firmware/defmt-rtt/README.md @@ -14,9 +14,7 @@ For more details about the framework check the book at https://defmt.ferrous-sys ## Customization -The RTT buffer size (default: 1024) can be configured with the `DEFMT_RTT_BUFFER_SIZE` environment variable. -- use a power of 2 for best performance -- caution: the `sccache` compiler cache is not supported because [it does not honor environment variables](https://github.com/mozilla/sccache/blob/master/docs/Rust.md). +The RTT buffer size (default: 1024) can be configured with the `DEFMT_RTT_BUFFER_SIZE` environment variable in a tight memory situation. Use a power of 2 for best performance. ## Support diff --git a/firmware/defmt-rtt/build.rs b/firmware/defmt-rtt/build.rs new file mode 100644 index 00000000..1075e077 --- /dev/null +++ b/firmware/defmt-rtt/build.rs @@ -0,0 +1,23 @@ +fn main() { + println!("cargo:rerun-if-env-changed=DEFMT_RTT_BUFFER_SIZE"); + + let size = option_env!("DEFMT_RTT_BUFFER_SIZE") + .map(|s| s.parse().ok()) + .flatten() + .unwrap_or(1024_usize); + + let non_power_of_two = (size & (size - 1)) != 0; + + if non_power_of_two { + println!("cargo:warning=RTT buffer size of {} is not a power of two, performance will be degraded.", size) + } + + let out_dir_path = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap()); + let out_file_path = out_dir_path.join("consts.rs"); + + std::fs::write( + out_file_path, + format!("pub(crate) const BUF_SIZE: usize = {};", size), + ) + .unwrap(); +} diff --git a/firmware/defmt-rtt/src/channel.rs b/firmware/defmt-rtt/src/channel.rs index ebe44bf7..3293bfe1 100644 --- a/firmware/defmt-rtt/src/channel.rs +++ b/firmware/defmt-rtt/src/channel.rs @@ -3,7 +3,7 @@ use core::{ sync::atomic::{AtomicUsize, Ordering}, }; -use crate::{MODE_BLOCK_IF_FULL, MODE_MASK, SIZE}; +use crate::{consts::BUF_SIZE, MODE_BLOCK_IF_FULL, MODE_MASK}; #[repr(C)] pub(crate) struct Channel { @@ -45,9 +45,9 @@ impl Channel { let available = if read > write { read - write - 1 } else if read == 0 { - SIZE - write - 1 + BUF_SIZE - write - 1 } else { - SIZE - write + BUF_SIZE - write }; if available == 0 { @@ -58,9 +58,9 @@ impl Channel { let len = bytes.len().min(available); unsafe { - if cursor + len > SIZE { + if cursor + len > BUF_SIZE { // split memcpy - let pivot = SIZE - cursor; + let pivot = BUF_SIZE - cursor; ptr::copy_nonoverlapping(bytes.as_ptr(), self.buffer.add(cursor), pivot); ptr::copy_nonoverlapping(bytes.as_ptr().add(pivot), self.buffer, len - pivot); } else { @@ -69,7 +69,7 @@ impl Channel { } } self.write - .store(write.wrapping_add(len) % SIZE, Ordering::Release); + .store(write.wrapping_add(len) % BUF_SIZE, Ordering::Release); len } @@ -77,13 +77,13 @@ impl Channel { fn nonblocking_write(&self, bytes: &[u8]) -> usize { let write = self.write.load(Ordering::Acquire); let cursor = write; - // NOTE truncate at SIZE to avoid more than one "wrap-around" in a single `write` call - let len = bytes.len().min(SIZE); + // NOTE truncate atBUF_SIZE to avoid more than one "wrap-around" in a single `write` call + let len = bytes.len().min(BUF_SIZE); unsafe { - if cursor + len > SIZE { + if cursor + len > BUF_SIZE { // split memcpy - let pivot = SIZE - cursor; + let pivot = BUF_SIZE - cursor; ptr::copy_nonoverlapping(bytes.as_ptr(), self.buffer.add(cursor), pivot); ptr::copy_nonoverlapping(bytes.as_ptr().add(pivot), self.buffer, len - pivot); } else { @@ -92,7 +92,7 @@ impl Channel { } } self.write - .store(write.wrapping_add(len) % SIZE, Ordering::Release); + .store(write.wrapping_add(len) % BUF_SIZE, Ordering::Release); len } diff --git a/firmware/defmt-rtt/src/consts.rs b/firmware/defmt-rtt/src/consts.rs new file mode 100644 index 00000000..849c10fc --- /dev/null +++ b/firmware/defmt-rtt/src/consts.rs @@ -0,0 +1,2 @@ +// see `build.rs` for contents +include!(concat!(env!("OUT_DIR"), "/consts.rs")); diff --git a/firmware/defmt-rtt/src/lib.rs b/firmware/defmt-rtt/src/lib.rs index 97f755db..7870806e 100644 --- a/firmware/defmt-rtt/src/lib.rs +++ b/firmware/defmt-rtt/src/lib.rs @@ -28,6 +28,12 @@ use cortex_m::{interrupt, register}; use crate::channel::Channel; +mod consts; + +/// RTT buffer size. Default: 1024; can be customized by setting `DEFMT_RTT_BUFFER_SIZE`. +/// Use a power of 2 for best performance. +use crate::consts::BUF_SIZE; + #[defmt::global_logger] struct Logger; @@ -94,20 +100,6 @@ const MODE_BLOCK_IF_FULL: usize = 2; /// Don't block if the RTT buffer is full. Truncate data to output as much as fits. const MODE_NON_BLOCKING_TRIM: usize = 1; -// buffer size. Default: 1024; can be customized by setting `DEFMT_RTT_BUFFER_SIZE` -// NOTE -// - use a power of 2 for best performance -// - `sccache` does not honor environment variables, so unset `RUSTC_WRAPPER` when customizing. -const SIZE: usize = size(); -const fn size() -> usize { - if let Some(size_s) = option_env!("DEFMT_RTT_BUFFER_SIZE") { - if let Ok(size) = konst::primitive::parse_usize(size_s) { - return size; - } - }; - 1024 -} - // make sure we only get shared references to the header/channel (avoid UB) /// # Safety /// `Channel` API is not re-entrant; this handle should not be held from different execution @@ -125,7 +117,7 @@ unsafe fn handle() -> &'static Channel { up_channel: Channel { name: NAME as *const _ as *const u8, buffer: unsafe { &mut BUFFER as *mut _ as *mut u8 }, - size: SIZE, + size: BUF_SIZE, write: AtomicUsize::new(0), read: AtomicUsize::new(0), flags: AtomicUsize::new(MODE_NON_BLOCKING_TRIM), @@ -134,7 +126,7 @@ unsafe fn handle() -> &'static Channel { #[cfg_attr(target_os = "macos", link_section = ".uninit,defmt-rtt.BUFFER")] #[cfg_attr(not(target_os = "macos"), link_section = ".uninit.defmt-rtt.BUFFER")] - static mut BUFFER: [u8; SIZE] = [0; SIZE]; + static mut BUFFER: [u8; BUF_SIZE] = [0; BUF_SIZE]; static NAME: &[u8] = b"defmt\0"; From 293930f629f6fed7cb0f8d8234c33e3ab3e50ef4 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 25 Nov 2021 18:44:00 +0100 Subject: [PATCH 4/7] remove dependency --- firmware/defmt-rtt/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/firmware/defmt-rtt/Cargo.toml b/firmware/defmt-rtt/Cargo.toml index 16c67799..bc25aa2f 100644 --- a/firmware/defmt-rtt/Cargo.toml +++ b/firmware/defmt-rtt/Cargo.toml @@ -12,5 +12,4 @@ version = "0.3.0" [dependencies] cortex-m = "0.7" -konst = "0.2" defmt = { version = "0.3", path = "../../defmt" } From b83d0d2277ba00198eb659f61790eb574f1be922 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 25 Nov 2021 18:44:45 +0100 Subject: [PATCH 5/7] dox++ --- firmware/defmt-rtt/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firmware/defmt-rtt/src/lib.rs b/firmware/defmt-rtt/src/lib.rs index 7870806e..023a66c1 100644 --- a/firmware/defmt-rtt/src/lib.rs +++ b/firmware/defmt-rtt/src/lib.rs @@ -30,7 +30,7 @@ use crate::channel::Channel; mod consts; -/// RTT buffer size. Default: 1024; can be customized by setting `DEFMT_RTT_BUFFER_SIZE`. +/// RTT buffer size. Default: 1024; can be customized by setting the `DEFMT_RTT_BUFFER_SIZE` environment variable. /// Use a power of 2 for best performance. use crate::consts::BUF_SIZE; From 957eab0d77dfd86c2ee698aa2861ef03d5f5a595 Mon Sep 17 00:00:00 2001 From: Anatol Ulrich Date: Thu, 25 Nov 2021 18:59:44 +0100 Subject: [PATCH 6/7] remove NPOT warning; organize imports --- firmware/defmt-rtt/build.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/firmware/defmt-rtt/build.rs b/firmware/defmt-rtt/build.rs index 1075e077..b01398cf 100644 --- a/firmware/defmt-rtt/build.rs +++ b/firmware/defmt-rtt/build.rs @@ -1,18 +1,16 @@ +use std::{env, path::PathBuf}; + fn main() { println!("cargo:rerun-if-env-changed=DEFMT_RTT_BUFFER_SIZE"); - let size = option_env!("DEFMT_RTT_BUFFER_SIZE") - .map(|s| s.parse().ok()) - .flatten() + let size = env::var("DEFMT_RTT_BUFFER_SIZE") + .map(|s| { + s.parse() + .expect("coult not parse DEFMT_RTT_BUFFER_SIZE as usize") + }) .unwrap_or(1024_usize); - let non_power_of_two = (size & (size - 1)) != 0; - - if non_power_of_two { - println!("cargo:warning=RTT buffer size of {} is not a power of two, performance will be degraded.", size) - } - - let out_dir_path = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap()); + let out_dir_path = PathBuf::from(env::var_os("OUT_DIR").unwrap()); let out_file_path = out_dir_path.join("consts.rs"); std::fs::write( From b441edb8e75969c81e28c7f030cedbeeea91b42c Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 25 Nov 2021 19:06:14 +0100 Subject: [PATCH 7/7] Update firmware/defmt-rtt/build.rs --- firmware/defmt-rtt/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firmware/defmt-rtt/build.rs b/firmware/defmt-rtt/build.rs index b01398cf..fa9aaf8d 100644 --- a/firmware/defmt-rtt/build.rs +++ b/firmware/defmt-rtt/build.rs @@ -6,7 +6,7 @@ fn main() { let size = env::var("DEFMT_RTT_BUFFER_SIZE") .map(|s| { s.parse() - .expect("coult not parse DEFMT_RTT_BUFFER_SIZE as usize") + .expect("could not parse DEFMT_RTT_BUFFER_SIZE as usize") }) .unwrap_or(1024_usize);