Skip to content

Commit

Permalink
remove leb128 / zigzag encoding.
Browse files Browse the repository at this point in the history
  • Loading branch information
Dirbaio committed Jun 13, 2021
1 parent 379f792 commit 21aff44
Show file tree
Hide file tree
Showing 13 changed files with 304 additions and 340 deletions.
2 changes: 1 addition & 1 deletion book/src/ser-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl Format for u8 {
fn format(&self, fmt: defmt::Formatter) {
if fmt.inner.needs_tag() {
let t = internp!("{=u8}");
fmt.inner.u8(&t);
fmt.inner.tag(&t);
}
fmt.inner.u8(self)
// on the wire: [1, 42]
Expand Down
29 changes: 7 additions & 22 deletions decoder/src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
}
}

let index = read_leb128(&mut self.bytes)?;
let index = self.bytes.read_u16::<LE>()? as usize;
let format = self
.table
.get_without_level(index as usize)
Expand Down Expand Up @@ -206,10 +206,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
Type::I32 => args.push(Arg::Ixx(self.bytes.read_i32::<LE>()? as i128)),
Type::I64 => args.push(Arg::Ixx(self.bytes.read_i64::<LE>()? as i128)),
Type::I128 => args.push(Arg::Ixx(self.bytes.read_i128::<LE>()?)),
// Signed isize is encoded in zigzag-encoding.
Type::Isize => args.push(Arg::Ixx(
zigzag_decode(read_leb128(&mut self.bytes)?) as i128
)),
Type::Isize => args.push(Arg::Ixx(self.bytes.read_i32::<LE>()? as i128)),
Type::U8 => args.push(Arg::Uxx(self.bytes.read_u8()? as u128)),
Type::U16 => args.push(Arg::Uxx(self.bytes.read_u16::<LE>()? as u128)),
Type::U24 => {
Expand All @@ -221,14 +218,14 @@ impl<'t, 'b> Decoder<'t, 'b> {
Type::U32 => args.push(Arg::Uxx(self.bytes.read_u32::<LE>()? as u128)),
Type::U64 => args.push(Arg::Uxx(self.bytes.read_u64::<LE>()? as u128)),
Type::U128 => args.push(Arg::Uxx(self.bytes.read_u128::<LE>()? as u128)),
Type::Usize => args.push(Arg::Uxx(read_leb128(&mut self.bytes)? as u128)),
Type::Usize => args.push(Arg::Uxx(self.bytes.read_u32::<LE>()? as u128)),
Type::F32 => args.push(Arg::F32(f32::from_bits(self.bytes.read_u32::<LE>()?))),
Type::F64 => args.push(Arg::F64(f64::from_bits(self.bytes.read_u64::<LE>()?))),
Type::Bool => args.push(Arg::Bool(Arc::new(Bool(AtomicBool::new(
self.bytes.read_u8()? != 0,
))))),
Type::FormatSlice => {
let num_elements = read_leb128(&mut self.bytes)? as usize;
let num_elements = self.bytes.read_u32::<LE>()? as usize;
let elements = self.decode_format_slice(num_elements)?;
args.push(Arg::FormatSlice { elements });
}
Expand Down Expand Up @@ -275,7 +272,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
args.push(Arg::Uxx(data));
}
Type::Str => {
let str_len = read_leb128(&mut self.bytes)? as usize;
let str_len = self.bytes.read_u32::<LE>()? as usize;
let mut arg_str_bytes = vec![];

// note: went for the suboptimal but simple solution; optimize if necessary
Expand All @@ -290,7 +287,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
args.push(Arg::Str(arg_str));
}
Type::IStr => {
let str_index = read_leb128(&mut self.bytes)? as usize;
let str_index = self.bytes.read_u16::<LE>()? as usize;

let string = self
.table
Expand All @@ -301,7 +298,7 @@ impl<'t, 'b> Decoder<'t, 'b> {
}
Type::U8Slice => {
// only supports byte slices
let num_elements = read_leb128(&mut self.bytes)? as usize;
let num_elements = self.bytes.read_u32::<LE>()? as usize;
let mut arg_slice = vec![];

// note: went for the suboptimal but simple solution; optimize if necessary
Expand Down Expand Up @@ -404,18 +401,6 @@ fn merge_bitfields(params: &mut Vec<Parameter>) {
params.append(&mut merged_bitfields);
}

pub fn read_leb128(bytes: &mut &[u8]) -> Result<u64, DecodeError> {
match leb128::read::unsigned(bytes) {
Ok(val) => Ok(val),
Err(leb128::read::Error::Overflow) => Err(DecodeError::Malformed),
Err(leb128::read::Error::IoError(io)) => Err(io.into()),
}
}

fn zigzag_decode(unsigned: u64) -> i64 {
(unsigned >> 1) as i64 ^ -((unsigned & 1) as i64)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
5 changes: 3 additions & 2 deletions decoder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ use std::{
},
};

use decoder::{read_leb128, Decoder};
use decoder::Decoder;
use defmt_parser::Level;
use elf2table::parse_impl;
use byteorder::{ReadBytesExt, LE};

pub use elf2table::{Location, Locations};
pub use frame::Frame;
Expand Down Expand Up @@ -188,7 +189,7 @@ impl Table {
mut bytes: &[u8],
) -> Result<(Frame<'t>, /*consumed: */ usize), DecodeError> {
let len = bytes.len();
let index = read_leb128(&mut bytes)?;
let index = bytes.read_u16::<LE>()? as u64;

let mut decoder = Decoder::new(self, bytes);

Expand Down
4 changes: 2 additions & 2 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,14 +993,14 @@ pub fn internp(ts: TokenStream) -> TokenStream {
let section_macos = mksection(true, "prim.", &sym);

if cfg!(feature = "unstable-test") {
quote!({ defmt::export::fetch_add_string_index() as u8 })
quote!({ defmt::export::fetch_add_string_index() as u16 })
} else {
quote!({
#[cfg_attr(target_os = "macos", link_section = #section_macos)]
#[cfg_attr(not(target_os = "macos"), link_section = #section)]
#[export_name = #sym]
static S: u8 = 0;
&S as *const u8 as u8
&S as *const u8 as u16
})
}
.into()
Expand Down
4 changes: 2 additions & 2 deletions src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<T: fmt::Debug + ?Sized> Format for Debug2Format<'_, T> {
fn format(&self, fmt: Formatter) {
if fmt.inner.needs_tag() {
let t = defmt_macros::internp!("{=__internal_Debug}");
fmt.inner.u8(&t);
fmt.inner.tag(&t);
}
fmt.inner.debug(&self.0);
}
Expand Down Expand Up @@ -65,7 +65,7 @@ impl<T: fmt::Display + ?Sized> Format for Display2Format<'_, T> {
fn format(&self, fmt: Formatter) {
if fmt.inner.needs_tag() {
let t = defmt_macros::internp!("{=__internal_Display}");
fmt.inner.u8(&t);
fmt.inner.tag(&t);
}
fmt.inner.display(&self.0);
}
Expand Down
18 changes: 8 additions & 10 deletions src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,22 @@ use crate::Str;

#[cfg(feature = "unstable-test")]
thread_local! {
static I: core::sync::atomic::AtomicU8 =
core::sync::atomic::AtomicU8::new(0);
static T: core::sync::atomic::AtomicU8 =
core::sync::atomic::AtomicU8::new(0);
static I: core::sync::atomic::AtomicU16 =
core::sync::atomic::AtomicU16::new(0);
static T: core::sync::atomic::AtomicU16 =
core::sync::atomic::AtomicU16::new(0);
}

// NOTE we limit these values to 7-bit to avoid LEB128 encoding while writing the expected answers
// in unit tests
/// For testing purposes
#[cfg(feature = "unstable-test")]
pub fn fetch_string_index() -> u8 {
I.with(|i| i.load(core::sync::atomic::Ordering::Relaxed)) & 0x7f
pub fn fetch_string_index() -> u16 {
I.with(|i| i.load(core::sync::atomic::Ordering::Relaxed))
}

/// For testing purposes
#[cfg(feature = "unstable-test")]
pub fn fetch_add_string_index() -> usize {
(I.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed)) & 0x7f) as usize
(I.with(|i| i.fetch_add(1, core::sync::atomic::Ordering::Relaxed))) as usize
}

#[cfg(feature = "unstable-test")]
Expand Down Expand Up @@ -186,7 +184,7 @@ mod sealed {
fn format(&self, fmt: Formatter) {
if fmt.inner.needs_tag() {
let t = internp!("Unwrap of a None option value");
fmt.inner.u8(&t);
fmt.inner.tag(&t);
}
}
}
Expand Down
27 changes: 9 additions & 18 deletions src/formatter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use core::fmt::{self, Write as _};

use crate::{export, leb, Format};
use crate::{export, Format};

/// Handle to a defmt logger.
pub struct Formatter<'a> {
Expand Down Expand Up @@ -72,11 +72,8 @@ impl InternalFormatter {
}

/// Implementation detail
/// leb64-encode `x` and write it to self.bytes
pub fn leb64(&mut self, x: usize) {
let mut buf: [u8; 10] = [0; 10];
let i = leb::leb64(x, &mut buf);
self.write(&buf[..i])
pub fn tag(&mut self, b: &u16) {
self.write(&b.to_le_bytes())
}

/// Implementation detail
Expand Down Expand Up @@ -106,13 +103,12 @@ impl InternalFormatter {

/// Implementation detail
pub fn isize(&mut self, b: &isize) {
// Zig-zag encode the signed value.
self.leb64(leb::zigzag_encode(*b));
self.write(&(*b as i32).to_le_bytes())
}

/// Implementation detail
pub fn fmt_slice(&mut self, values: &[impl Format]) {
self.leb64(values.len());
self.usize(&values.len());
let mut is_first = true;
for value in values {
let omit_tag = !is_first;
Expand Down Expand Up @@ -159,7 +155,7 @@ impl InternalFormatter {

/// Implementation detail
pub fn usize(&mut self, b: &usize) {
self.leb64(*b);
self.write(&(*b as u32).to_le_bytes())
}

/// Implementation detail
Expand All @@ -173,12 +169,12 @@ impl InternalFormatter {
}

pub fn str(&mut self, s: &str) {
self.leb64(s.len());
self.usize(&s.len());
self.write(s.as_bytes());
}

pub fn slice(&mut self, s: &[u8]) {
self.leb64(s.len());
self.usize(&s.len());
self.write(s);
}

Expand All @@ -199,12 +195,7 @@ impl InternalFormatter {

/// Implementation detail
pub fn istr(&mut self, s: &Str) {
// LEB128 encoding
if s.address < 128 {
self.write(&[s.address as u8])
} else {
self.write(&[s.address as u8 | (1 << 7), (s.address >> 7) as u8])
}
self.write(&s.address.to_le_bytes())
}

/// Implementation detail
Expand Down
2 changes: 1 addition & 1 deletion src/impls/arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ macro_rules! arrays {
fn format(&self, fmt: Formatter) {
if fmt.inner.needs_tag() {
let t = internp!($fmt);
fmt.inner.u8(&t);
fmt.inner.tag(&t);
}
fmt.inner.fmt_array(self);
}
Expand Down
6 changes: 3 additions & 3 deletions src/impls/core_/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ where
fn format(&self, f: Formatter) {
if f.inner.needs_tag() {
let t = internp!("None|Some({=?})");
f.inner.u8(&t);
f.inner.tag(&t);
}
match self {
None => f.inner.u8(&0),
Expand All @@ -37,7 +37,7 @@ where
fn format(&self, f: Formatter) {
if f.inner.needs_tag() {
let t = internp!("Err({=?})|Ok({=?})");
f.inner.u8(&t);
f.inner.tag(&t);
}
match self {
Err(e) => {
Expand All @@ -56,7 +56,7 @@ impl<T> Format for core::marker::PhantomData<T> {
fn format(&self, f: Formatter) {
if f.inner.needs_tag() {
let t = internp!("PhantomData");
f.inner.u8(&t);
f.inner.tag(&t);
}
}
}
Expand Down
Loading

0 comments on commit 21aff44

Please sign in to comment.