From 8ae2f9c42c8ec4ee987a0847ffe725d577a6eead Mon Sep 17 00:00:00 2001 From: irobot Date: Fri, 5 Dec 2025 23:59:25 -0800 Subject: [PATCH 1/3] Parse the temperature in reverse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting from the end of the line, take the digits one at a time. For example the following line: > Seattle;-12.3 * Temperature * 10 (T) is encoded as an `i16`. * Starting from the end, the first digit is 3. T = 3 * Skip over the dot, T = 3 * Next is 2. T = 2 * 10 + 3 = 23 * Next character could be a digit, but it could also be - or ; In this case it is 1, so T = 1 * 100 + T = 123 * One last time, we need to check for a -, which we have, so T = -T = -123 * Next character is guaranteed to be ; thus we have also obtained the length of the station name. With this change, Hyperfine reports a ~1.50 faster finish time. Tested on a 12 core / 24 thread AMD Ryzen 5900x. > Benchmark 1: target/release/brrr-org Time (mean ± σ): 2.205 s ± 0.020 s [User: 42.833 s, System: 1.119 s] Range (min … max): 2.194 s … 2.260 s 10 runs Benchmark 2: target/release/brrr Time (mean ± σ): 1.473 s ± 0.021 s [User: 25.448 s, System: 1.108 s] Range (min … max): 1.452 s … 1.529 s 10 runs Summary target/release/brrr ran 1.50 ± 0.03 times faster than target/release/brrr-org --- src/main.rs | 79 ++++++++++++++++++++++++----------------------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/src/main.rs b/src/main.rs index 302d0bb..d3b3e84 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,7 +7,7 @@ use std::io::Write; use std::{ borrow::Borrow, - collections::{BTreeMap, HashMap, btree_map::Entry}, + collections::{btree_map::Entry, BTreeMap, HashMap}, ffi::{c_int, c_void}, fs::File, hash::{BuildHasher, Hash, Hasher}, @@ -15,7 +15,6 @@ use std::{ simd::{cmp::SimdPartialEq, u8x64}, }; -const SEMI: u8x64 = u8x64::splat(b';'); const NEWL: u8x64 = u8x64::splat(b'\n'); struct FastHasherBuilder; @@ -235,11 +234,40 @@ fn one(map: &[u8]) -> HashMap { let newline_at = at + next_newline(map, at); let line = unsafe { map.get_unchecked(at..newline_at) }; at = newline_at + 1; - let semi = semi_at(line); - let station = unsafe { line.get_unchecked(..semi) }; - let temperature = unsafe { line.get_unchecked(semi + 1..) }; - let t = parse_temperature(temperature); - update_stats(&mut stats, station, t); + // parse temperature backwards, avoiding search for semicolon + let mut index = line.len() - 1; + // The first digit after the decimal point (10^-1) + let mut temperature = unsafe { *line.get_unchecked(index) - b'0' } as i16; + // skip over the dot (.) + index -= 2; + + // The digit before the dot (10^0) + temperature += (unsafe { *line.get_unchecked(index) - b'0' } as i16) * 10; + + index -= 1; + match *unsafe { line.get_unchecked(index) } { + // No minus sign and the number is < 10 + b';' => (), + // Minus sign but the number is > -10 + b'-' => { + temperature = -temperature; + index -= 1; + } + // The number is > 10 (so far) + digit => { + temperature += ((digit - b'0') as i16) * 100; + index -= 1; + } + } + // This character can only be either ';' or '-' + if unsafe { *line.get_unchecked(index) } == b'-' { + // The number is negative and <= -10 + temperature = -temperature; + index -= 1; + } + // `index` now points at the semicolon + let station = unsafe { line.get_unchecked(..index) }; + update_stats(&mut stats, station, temperature); } stats } @@ -292,43 +320,6 @@ fn next_newline(map: &[u8], at: usize) -> usize { } } -#[inline] -fn semi_at(line: &[u8]) -> usize { - // we know, line is at most 100+1+5 = 106b - if line.len() > 64 { - std::hint::cold_path(); - line.iter().position(|c| *c == b';').unwrap() - } else { - let delim_eq = SEMI.simd_eq(u8x64::load_or_default(line)); - // SAFETY: we're promised there is a ; in every line - unsafe { delim_eq.first_set().unwrap_unchecked() } - } -} - -#[inline] -fn parse_temperature(t: &[u8]) -> i16 { - let tlen = t.len(); - unsafe { std::hint::assert_unchecked(tlen >= 3) }; - let is_neg = std::hint::select_unpredictable(t[0] == b'-', true, false); - let sign = i16::from(!is_neg) * 2 - 1; - let skip = usize::from(is_neg); - let has_dd = std::hint::select_unpredictable(tlen - skip == 4, true, false); - let mul = i16::from(has_dd) * 90 + 10; - let t1 = mul * i16::from(t[skip] - b'0'); - let t2 = i16::from(has_dd) * 10 * i16::from(t[tlen - 3] - b'0'); - let t3 = i16::from(t[tlen - 1] - b'0'); - sign * (t1 + t2 + t3) -} - -#[test] -fn pt() { - assert_eq!(parse_temperature(b"0.0"), 0); - assert_eq!(parse_temperature(b"9.2"), 92); - assert_eq!(parse_temperature(b"-9.2"), -92); - assert_eq!(parse_temperature(b"98.2"), 982); - assert_eq!(parse_temperature(b"-98.2"), -982); -} - fn mmap(f: &File) -> &'_ [u8] { let len = f.metadata().unwrap().len(); unsafe { From 2571c5286787087f7d99b730ce7067b61acd6e23 Mon Sep 17 00:00:00 2001 From: irobot Date: Sat, 6 Dec 2025 15:27:37 -0800 Subject: [PATCH 2/3] Reformat to more closely match main branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit + move temperature parsing code to parse_temperature + restore temp parsing unit tests + replace usize decrement with unchecked_sub() just in case, though it doesn't seem to make any meaningful difference perf-wise. After merging in the latest improvements, the speed up offered by this change is no longer as significant. ``` Benchmark 1: target/release/brrr-main Time (mean ± σ): 1.196 s ± 0.006 s [User: 19.190 s, System: 1.133 s] Range (min … max): 1.186 s … 1.207 s 10 runs Benchmark 2: target/release/brrr Time (mean ± σ): 1.143 s ± 0.005 s [User: 17.915 s, System: 1.150 s] Range (min … max): 1.135 s … 1.150 s 10 runs Summary target/release/brrr ran 1.05 ± 0.01 times faster than target/release/brrr-main ``` --- src/main.rs | 108 +++++++++++++++++++++------------------------------- 1 file changed, 43 insertions(+), 65 deletions(-) diff --git a/src/main.rs b/src/main.rs index c6e5523..94b8933 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,6 @@ use std::io::Write; use std::{ borrow::Borrow, collections::{btree_map::Entry, BTreeMap, HashMap}, - ffi::{c_int, c_void}, fs::File, hash::{BuildHasher, Hash, Hasher}, mem::ManuallyDrop, @@ -265,40 +264,8 @@ fn one(map: &[u8]) -> HashMap { let newline_at = at + unsafe { find_newline(&map[at..]).unwrap_unchecked() }; let line = unsafe { map.get_unchecked(at..newline_at) }; at = newline_at + 1; - // parse temperature backwards, avoiding search for semicolon - let mut index = line.len() - 1; - // The first digit after the decimal point (10^-1) - let mut temperature = unsafe { *line.get_unchecked(index) - b'0' } as i16; - // skip over the dot (.) - index -= 2; - - // The digit before the dot (10^0) - temperature += (unsafe { *line.get_unchecked(index) - b'0' } as i16) * 10; - - index -= 1; - match *unsafe { line.get_unchecked(index) } { - // No minus sign and the number is < 10 - b';' => (), - // Minus sign but the number is > -10 - b'-' => { - temperature = -temperature; - index -= 1; - } - // The number is > 10 (so far) - digit => { - temperature += ((digit - b'0') as i16) * 100; - index -= 1; - } - } - // This character can only be either ';' or '-' - if unsafe { *line.get_unchecked(index) } == b'-' { - // The number is negative and <= -10 - temperature = -temperature; - index -= 1; - } - // `index` now points at the semicolon - let station = unsafe { line.get_unchecked(..index) }; - update_stats(&mut stats, station, temperature); + let (t, station) = parse_line(line); + update_stats(&mut stats, station, t); } stats } @@ -318,19 +285,6 @@ fn update_stats(stats: &mut HashMap, station: & stats.count += 1; } -// SAFETY: buffer must contain a semicolon in the last min(8, buffer.len()) bytes -unsafe fn split_at_semicolon(buffer: &[u8]) -> (&[u8], &[u8]) { - let mut pos = buffer.len() - 4; - unsafe { - // SAFETY: readme promises there will be a semicolon - while *buffer.get_unchecked(pos) != b';' { - pos -= 1; - } - let (before, after) = buffer.split_at_unchecked(pos + 1); - (&before[..before.len() - 1], after) - } -} - pub fn find_newline(mut buffer: &[u8]) -> Option { const LANES: usize = 32; const SPLAT: Simd = Simd::splat(b'\n'); @@ -351,27 +305,51 @@ pub fn find_newline(mut buffer: &[u8]) -> Option { } #[inline] -fn parse_temperature(t: &[u8]) -> i16 { - let tlen = t.len(); - unsafe { std::hint::assert_unchecked(tlen >= 3) }; - let is_neg = std::hint::select_unpredictable(t[0] == b'-', true, false); - let sign = i16::from(!is_neg) * 2 - 1; - let skip = usize::from(is_neg); - let has_dd = std::hint::select_unpredictable(tlen - skip == 4, true, false); - let mul = i16::from(has_dd) * 90 + 10; - let t1 = mul * i16::from(t[skip] - b'0'); - let t2 = i16::from(has_dd) * 10 * i16::from(t[tlen - 3] - b'0'); - let t3 = i16::from(t[tlen - 1] - b'0'); - sign * (t1 + t2 + t3) +fn parse_line(line: &[u8]) -> (i16, &[u8]) { + // parse temperature backwards, avoiding search for semicolon + let mut index = line.len() - 1; + // The first digit after the decimal point (10^-1) + let mut temperature = unsafe { *line.get_unchecked(index) - b'0' } as i16; + // skip over the dot (.) + index = unsafe { index.unchecked_sub(2) }; + + // The digit before the dot (10^0) + temperature += (unsafe { *line.get_unchecked(index) - b'0' } as i16) * 10; + + index = unsafe { index.unchecked_sub(1) }; + match *unsafe { line.get_unchecked(index) } { + // No minus sign and the number is < 10 + b';' => (), + // Minus sign but the number is > -10 + b'-' => { + temperature = -temperature; + index = unsafe { index.unchecked_sub(1) }; + } + // The number is > 10 (so far) + digit => { + temperature += ((digit - b'0') as i16) * 100; + index = unsafe { index.unchecked_sub(1) }; + } + } + // This character can only be either ';' or '-' + if unsafe { *line.get_unchecked(index) } == b'-' { + // The number is negative and <= -10 + temperature = -temperature; + index = unsafe { index.unchecked_sub(1) }; + } + // `index` now points at the semicolon + let station = unsafe { line.get_unchecked(..index) }; + (temperature, station) } #[test] fn pt() { - assert_eq!(parse_temperature(b"0.0"), 0); - assert_eq!(parse_temperature(b"9.2"), 92); - assert_eq!(parse_temperature(b"-9.2"), -92); - assert_eq!(parse_temperature(b"98.2"), 982); - assert_eq!(parse_temperature(b"-98.2"), -982); + // parse_line returns the temperature and station. + assert_eq!(parse_line(b"Seattle;0.0"), (0, "Seattle".as_bytes())); + assert_eq!(parse_line(b"Phoenix;9.2"), (92, "Phoenix".as_bytes())); + assert_eq!(parse_line(b"Bergen;-9.2"), (-92, "Bergen".as_bytes())); + assert_eq!(parse_line(b"Athens;98.2"), (982, "Athens".as_bytes())); + assert_eq!(parse_line(b"Tampere;-98.2"), (-982, "Tampere".as_bytes())); } #[cfg(unix)] From df4a6fcee7e2ba3efd7ddfbaf8b9a8ce84d8f787 Mon Sep 17 00:00:00 2001 From: irobot Date: Sun, 7 Dec 2025 15:59:36 -0800 Subject: [PATCH 3/3] Rustfmt style edition 2024? Wondering if CI failing has to do with rustfmt using a different style edition. --- src/main.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index c8c572d..744ea2c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,11 +7,11 @@ use std::io::Write; use std::{ borrow::Borrow, - collections::{btree_map::Entry, BTreeMap, HashMap}, + collections::{BTreeMap, HashMap, btree_map::Entry}, fs::File, hash::{BuildHasher, Hash, Hasher}, mem::ManuallyDrop, - simd::{cmp::SimdPartialEq, Simd}, + simd::{Simd, cmp::SimdPartialEq}, }; #[cfg(unix)] @@ -380,7 +380,7 @@ fn mmap(f: &File) -> &'_ [u8] { fn mmap(f: &File) -> &'_ [u8] { use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::System::Memory::{ - CreateFileMappingW, MapViewOfFile, FILE_MAP_READ, PAGE_READONLY, + CreateFileMappingW, FILE_MAP_READ, MapViewOfFile, PAGE_READONLY, }; let len = f.metadata().unwrap().len();