diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e2f220..b1bfdaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All significant changes to this software be documented in this file. ## Unreleased +### Bug fixes + +* Parse arbitrarily precise fractional byte sizes without double rounding or false overflow. + ## v0.3.0 (2026-06-28) ### Breaking changes diff --git a/Cargo.lock b/Cargo.lock index 0ab4924..ac5c2de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -71,6 +71,7 @@ checksum = "b4388bee8683e3d04af747c73422af53102d2bd24d9eadb6cbc100baef4b43f8" name = "bsize" version = "0.3.0" dependencies = [ + "divan", "insta", "macroweave", "quickcheck", @@ -106,6 +107,7 @@ dependencies = [ "anstyle", "clap_lex", "strsim", + "terminal_size", ] [[package]] @@ -132,6 +134,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "condtype" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf0a07a401f374238ab8e2f11a104d2851bf9ce711ec69804834de8af45c7af" + [[package]] name = "console" version = "0.16.3" @@ -143,6 +151,31 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "divan" +version = "0.1.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a405457ec78b8fe08b0e32b4a3570ab5dff6dd16eb9e76a5ee0a9d9cbd898933" +dependencies = [ + "cfg-if", + "clap", + "condtype", + "divan-macros", + "libc", + "regex-lite", +] + +[[package]] +name = "divan-macros" +version = "0.1.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9556bc800956545d6420a640173e5ba7dfa82f38d3ea5a167eb555bc69ac3323" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "encode_unicode" version = "1.0.0" @@ -369,6 +402,12 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "regex-lite" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cab834c73d247e67f4fae452806d17d3c7501756d98c8808d7c9c7aa7d18f973" + [[package]] name = "regex-syntax" version = "0.8.11" @@ -482,6 +521,16 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "terminal_size" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" +dependencies = [ + "rustix", + "windows-sys", +] + [[package]] name = "toml" version = "1.1.2+spec-1.1.0" diff --git a/bsize/Cargo.toml b/bsize/Cargo.toml index 743281f..22a0a2a 100644 --- a/bsize/Cargo.toml +++ b/bsize/Cargo.toml @@ -42,11 +42,16 @@ macroweave = "0.1.0" serde_core = { version = "1", default-features = false, optional = true } [dev-dependencies] +divan = "0.1.21" insta = { version = "1.47.2" } quickcheck = { version = "1.0.0" } serde = { version = "1.0.228", features = ["derive"] } serde_json = { version = "1.0.150" } toml = { version = "1.1.2" } +[[bench]] +harness = false +name = "parse" + [lints] workspace = true diff --git a/bsize/benches/parse.rs b/bsize/benches/parse.rs new file mode 100644 index 0000000..dce3ba0 --- /dev/null +++ b/bsize/benches/parse.rs @@ -0,0 +1,59 @@ +// Copyright 2026 FastLabs Developers +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::fmt; + +use bsize::BSize64; +use bsize::ParseError; + +#[derive(Clone, Copy)] +struct ParseCase { + name: &'static str, + input: &'static str, +} + +impl fmt::Display for ParseCase { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.name) + } +} + +const fn case(name: &'static str, input: &'static str) -> ParseCase { + ParseCase { name, input } +} + +const CASES: [ParseCase; 10] = [ + case("plain", "42"), + case("decimal-unit", "42 MB"), + case("binary-unit", "1 KiB"), + case("fraction", "1.5 MiB"), + case("small-fraction", "0.0025 KB"), + case("grouped", "1_234_567_890"), + case("u64-max", "18_446_744_073_709_551_615"), + case("high-precision-decimal", "1.84467440737095516145 EB"), + case( + "high-precision-binary", + "0.0000000000000000004336808689942017736029811203479766845703125 EiB", + ), + case("malformed", "not-a-size"), +]; + +fn main() { + divan::main(); +} + +#[divan::bench(args = CASES, sample_size = 1024)] +fn parse(case: ParseCase) -> Result { + divan::black_box(case.input).parse() +} diff --git a/bsize/src/parse.rs b/bsize/src/parse.rs index 5fb2abb..78c8cc8 100644 --- a/bsize/src/parse.rs +++ b/bsize/src/parse.rs @@ -62,9 +62,6 @@ where .map_err(|_| ParseError::Overflow) } -// This is derived from `parse-size` [1]. -// -// [1]: https://github.com/kennytm/parse-size/blob/8f2bc5a8/src/lib.rs#L364-L495 fn parse_size(mut src: &[u8]) -> Result { // trim starting and trailing spaces while let [b' ', init @ ..] = src { @@ -79,17 +76,17 @@ fn parse_size(mut src: &[u8]) -> Result { src = init; }; - let mut multiply = 1u64; + let mut multiplier = 1u64; if let [init @ .., b'i' | b'I'] = src { src = init; if let [init @ .., prefix] = src { match prefix { - b'k' | b'K' => multiply = 1 << 10, - b'm' | b'M' => multiply = 1 << 20, - b'g' | b'G' => multiply = 1 << 30, - b't' | b'T' => multiply = 1 << 40, - b'p' | b'P' => multiply = 1 << 50, - b'e' | b'E' => multiply = 1 << 60, + b'k' | b'K' => multiplier = 1 << 10, + b'm' | b'M' => multiplier = 1 << 20, + b'g' | b'G' => multiplier = 1 << 30, + b't' | b'T' => multiplier = 1 << 40, + b'p' | b'P' => multiplier = 1 << 50, + b'e' | b'E' => multiplier = 1 << 60, _ => return Err(ParseError::Malformed), } @@ -102,12 +99,12 @@ fn parse_size(mut src: &[u8]) -> Result { if let [init @ .., prefix] = src { 'skip: { match prefix { - b'k' | b'K' => multiply = 1_000, - b'm' | b'M' => multiply = 1_000_000, - b'g' | b'G' => multiply = 1_000_000_000, - b't' | b'T' => multiply = 1_000_000_000_000, - b'p' | b'P' => multiply = 1_000_000_000_000_000, - b'e' | b'E' => multiply = 1_000_000_000_000_000_000, + b'k' | b'K' => multiplier = 1_000, + b'm' | b'M' => multiplier = 1_000_000, + b'g' | b'G' => multiplier = 1_000_000_000, + b't' | b'T' => multiplier = 1_000_000_000_000, + b'p' | b'P' => multiplier = 1_000_000_000_000_000, + b'e' | b'E' => multiplier = 1_000_000_000_000_000_000, _ => break 'skip, } src = init; @@ -120,85 +117,64 @@ fn parse_size(mut src: &[u8]) -> Result { src = init; } - macro_rules! append_digit { - ($before:expr, $method:ident, $digit_char:expr) => { - $before - .checked_mul(10) - .and_then(|v| v.$method(($digit_char - b'0').into())) - }; - } - - #[derive(Debug, Clone, Copy, PartialEq, Eq)] - enum ParseState { - Empty, - Integer, - IntegerOverflow, - Fraction, - FractionOverflow, - } + let mut integer = 0u64; + let mut saw_digit = false; + let mut fraction_start = None; - let mut mantissa = 0u64; - let mut exponent = 0i32; - let mut state = ParseState::Empty; - - for b in src { - match (state, *b) { - (ParseState::Integer | ParseState::Empty, b'0'..=b'9') => { - if let Some(m) = append_digit!(mantissa, checked_add, *b) { - mantissa = m; - state = ParseState::Integer; - } else { - if *b >= b'5' { - mantissa = mantissa.checked_add(1).ok_or(ParseError::Overflow)?; - } - state = ParseState::IntegerOverflow; - exponent += 1; + for (index, b) in src.iter().copied().enumerate() { + match b { + b'0'..=b'9' => { + saw_digit = true; + if fraction_start.is_none() { + integer = integer + .checked_mul(10) + .and_then(|v| v.checked_add(u64::from(b - b'0'))) + .ok_or(ParseError::Overflow)?; } } - (ParseState::IntegerOverflow, b'0'..=b'9') => { - exponent += 1; - } - (ParseState::Fraction, b'0'..=b'9') => { - if let Some(m) = append_digit!(mantissa, checked_add, *b) { - mantissa = m; - exponent -= 1; - } else { - if *b >= b'5' { - mantissa = mantissa.checked_add(1).ok_or(ParseError::Overflow)?; - } - state = ParseState::FractionOverflow; - } + b'_' => {} + b'.' if saw_digit && fraction_start.is_none() => { + fraction_start = Some(index + 1); } - (_, b'_') => {} - (ParseState::Integer, b'.') => state = ParseState::Fraction, - (ParseState::IntegerOverflow, b'.') => state = ParseState::FractionOverflow, _ => return Err(ParseError::Malformed), } } - if matches!(state, ParseState::Empty) { + if !saw_digit { return Err(ParseError::Empty); } - let abs_exponent = exponent.unsigned_abs(); - if exponent >= 0 { - let power = 10_u64 - .checked_pow(abs_exponent) - .ok_or(ParseError::Overflow)?; - let multiply = multiply.checked_mul(power).ok_or(ParseError::Overflow)?; - mantissa.checked_mul(multiply).ok_or(ParseError::Overflow) - } else if exponent >= -38 { - let power = 10_u128.pow(abs_exponent); - let result = (u128::from(mantissa) * u128::from(multiply) + power / 2) / power; - u64::try_from(result).map_err(|_| ParseError::Overflow) - } else { - // (2^128) * 1e-39 < 1, always, and thus saturate to 0. - Ok(0) + let mut bytes = integer + .checked_mul(multiplier) + .ok_or(ParseError::Overflow)?; + + if let Some(start) = fraction_start { + // Multiply the fraction by the unit multiplier from right to left in base 10. + // Once all fractional digits are consumed, carry is the integral byte count and + // the last remainder digit determines rounding to the nearest byte. + debug_assert!(multiplier <= u64::MAX / 10); + let mut carry = 0u64; + let mut rounding_digit = 0u64; + for b in src[start..].iter().copied().rev() { + if b == b'_' { + continue; + } + + let product = u64::from(b - b'0') * multiplier + carry; + rounding_digit = product % 10; + carry = product / 10; + } + + let fraction = carry + u64::from(rounding_digit >= 5); + bytes = bytes.checked_add(fraction).ok_or(ParseError::Overflow)?; } + + Ok(bytes) } #[cfg(test)] mod tests { + use alloc::format; use alloc::string::ToString; use super::*; @@ -262,9 +238,22 @@ mod tests { ("0.0025KB", 3), ("0.4B", 0), ("0.5B", 1), + ("0.1234567890123456789012", 0), + ("1.84467440737095516155", 2), + ("1.84467440737095516145 EB", 1_844_674_407_370_955_161), + ("1.844674407370955161450 EB", 1_844_674_407_370_955_161), + ( + "0.0000000000000000004336808689942017736029811203479766845703124 EiB", + 0, + ), + ( + "0.0000000000000000004336808689942017736029811203479766845703125 EiB", + 1, + ), ("18_446_744_073_709_551_581", 18_446_744_073_709_551_581), ("18_446_744_073_709_551_615", u64::MAX), ("18.446_744_073_709_551_615 EB", u64::MAX), + ("18.4467440737095516154 EB", u64::MAX), ("1.000_000_000_000_000_001 EB", 1_000_000_000_000_000_001), ] { assert_parse_ok(input, expected); @@ -295,7 +284,6 @@ mod tests { "1 YiB", "1e2 KIB", "1E+6", - "0.1234567890123456789012", "\t1", "1\tKB", ] { @@ -308,6 +296,7 @@ mod tests { "18446744073709551615.5", "184467440737095516155", "18.446_744_073_709_551_616 EB", + "18.4467440737095516155 EB", "19EB", "16EiB", "100000000000000000000", @@ -319,4 +308,35 @@ mod tests { assert_eq!("64 KiB".parse::>(), Err(ParseError::Overflow)); assert_eq!("4GiB".parse::>(), Err(ParseError::Overflow)); } + + quickcheck::quickcheck! { + fn parses_eib_fractions_exactly(whole: u8, fraction: u64) -> bool { + const MULTIPLIER: u128 = 1 << 60; + const SCALE: u128 = 1_000_000_000_000_000_000; + + let whole = whole % 16; + let fraction = fraction % SCALE as u64; + let input = format!("{whole}.{fraction:018} EiB"); + let actual = input.parse::>(); + let expected = u128::from(whole) * MULTIPLIER + + (u128::from(fraction) * MULTIPLIER + SCALE / 2) / SCALE; + + if expected > u128::from(u64::MAX) { + actual == Err(ParseError::Overflow) + } else { + actual == Ok(ByteSize::b(u64::try_from(expected).unwrap())) + } + } + + fn fractional_trailing_zero_preserves_value(whole: u8, fraction: u64) -> bool { + const SCALE: u64 = 1_000_000_000_000_000_000; + + let whole = whole % 16; + let fraction = fraction % SCALE; + let input = format!("{whole}.{fraction:018} EiB"); + let input_with_zero = format!("{whole}.{fraction:018}0 EiB"); + + input.parse::>() == input_with_zero.parse::>() + } + } } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 4096d39..6f0d048 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -31,6 +31,7 @@ struct Command { impl Command { fn run(self) { match self.sub { + SubCommand::Bench(cmd) => cmd.run(), SubCommand::Lint(cmd) => cmd.run(), SubCommand::Test(cmd) => cmd.run(), } @@ -39,12 +40,23 @@ impl Command { #[derive(Subcommand)] enum SubCommand { + #[clap(about = "Run workspace benchmarks.")] + Bench(CommandBench), #[clap(about = "Run workspace quality checks.")] Lint(CommandLint), #[clap(about = "Run workspace unit tests.")] Test(CommandTest), } +#[derive(Parser)] +struct CommandBench; + +impl CommandBench { + fn run(self) { + run_command(make_bench_cmd()); + } +} + #[derive(Parser)] struct CommandTest { #[arg(long, help = "Run tests serially and do not capture output.")] @@ -121,6 +133,12 @@ fn make_test_cmd(no_capture: bool, features: &[&str]) -> StdCommand { cmd } +fn make_bench_cmd() -> StdCommand { + let mut cmd = find_command("cargo"); + cmd.args(["bench", "--workspace", "--bench", "*"]); + cmd +} + fn make_format_cmd(fix: bool) -> StdCommand { let mut cmd = find_command("cargo"); cmd.args(["+nightly", "fmt", "--all"]);