Skip to content

Commit ac9e84a

Browse files
BytesIter to read by index instead of re-slicing per byte (the VecTryIter approach) (#1935)
### What Changed the implementation of `BytesIter` to match the more efficient implementation of `VecTryIter`. ### Why This reduces the need to allocate memory and increases performance. This is benchmarked on [dkcumming/rs-soroban-sdk branch dc/bytes-from-zc-bench](main...dkcumming:rs-soroban-sdk:dc/bytes-iter-bench). For completeness the key end-to-end results are communicated in this table: | Bench | Before CPU | After CPU | Δ % | Before Mem | After Mem | Δ % | |-------------------|-----------:|----------:|-------:|-----------:|----------:|------:| | `baseline_*` | 245,345 | 232,140 | -5.4% | 1,146,818 | 1,144,946 | -0.2% | | `iter_32` | 386,297 | 258,596 | -33.1% | 1,151,154 | 1,144,946 | -0.5% | | `iter_rev_32` | 364,213 | 259,068 | -28.9% | 1,151,154 | 1,144,946 | -0.5% | | `iter_48` | 456,057 | 271,268 | -40.5% | 1,153,706 | 1,144,946 | -0.8% | | `iter_rev_48` | 422,837 | 271,740 | -35.7% | 1,153,706 | 1,144,946 | -0.8% | | `iter_96` | 665,721 | 309,284 | -53.5% | 1,162,898 | 1,144,946 | -1.5% | | `iter_rev_96` | 599,093 | 309,756 | -48.3% | 1,162,898 | 1,144,946 | -1.5% | | `iter_192` | 1,086,777 | 385,316 | -64.5% | 1,188,194 | 1,144,946 | -3.6% | | `iter_rev_192` | 953,333 | 385,788 | -59.5% | 1,188,194 | 1,144,946 | -3.6% | The tests to run these benchmarks were: <details> The contract: ```rust #![no_std] use soroban_sdk::{contract, contractimpl, Bytes, Env}; #[contract] pub struct Contract; // Each function derives its result from every byte so the iteration cannot be // optimized out of the guest Wasm. // // `baseline` has the same signature as the iterating functions, so subtracting // it isolates the iteration cost from dispatch and operand-passing overhead. #[contractimpl] impl Contract { pub fn baseline(_env: Env, _b: Bytes) -> u32 { 0 } pub fn sum_iter(_env: Env, b: Bytes) -> u32 { let mut acc: u32 = 0; for x in b.iter() { acc = acc.wrapping_add(x as u32); } acc } pub fn sum_iter_rev(_env: Env, b: Bytes) -> u32 { let mut acc: u32 = 0; for x in b.iter().rev() { acc = acc.wrapping_add(x as u32); } acc } } ``` The benchmark harnesses: ```rust //! Compute-budget microbenches for iterating a `Bytes` (host-function metering //! only). //! //! These benches are disabled by default (`#[ignore]`) because: //! 1. they should be run under `--release` rather than the default test profile. //! 2. their output is only meaningful with `--nocapture`. //! //! Run with the following command: //! cargo test --release --package soroban-sdk --lib --features testutils \ //! -- tests::bytes_iter_bench --ignored --nocapture use crate::{Bytes, Env}; fn report(label: &str, env: &Env) { let cpu = env.cost_estimate().budget().cpu_instruction_cost(); let mem = env.cost_estimate().budget().memory_bytes_cost(); println!("BENCH {label} cpu={cpu} mem={mem}"); } macro_rules! bench_iter { ($fwd:ident, $rev:ident, $n:literal) => { #[test] #[ignore] fn $fwd() { let env = Env::default(); let b = Bytes::from_slice(&env, &[7u8; $n]); env.cost_estimate().budget().reset_unlimited(); let mut acc: u32 = 0; for x in b.iter() { acc = acc.wrapping_add(x as u32); } core::hint::black_box(acc); report(stringify!($fwd), &env); } #[test] #[ignore] fn $rev() { let env = Env::default(); let b = Bytes::from_slice(&env, &[7u8; $n]); env.cost_estimate().budget().reset_unlimited(); let mut acc: u32 = 0; for x in b.iter().rev() { acc = acc.wrapping_add(x as u32); } core::hint::black_box(acc); report(stringify!($rev), &env); } }; } bench_iter!(bench_iter_32, bench_iter_rev_32, 32); bench_iter!(bench_iter_48, bench_iter_rev_48, 48); bench_iter!(bench_iter_96, bench_iter_rev_96, 96); bench_iter!(bench_iter_192, bench_iter_rev_192, 192); ``` </details> ### Known limitations No known limitations with this approach. This was a targeted investigation and is not comprehensive in scope, there may be other places where similar improvements can be made. Co-authored-by: Leigh <351529+leighmcculloch@users.noreply.github.com>
1 parent 28142cd commit ac9e84a

1 file changed

Lines changed: 98 additions & 28 deletions

File tree

soroban-sdk/src/bytes.rs

Lines changed: 98 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -839,57 +839,53 @@ impl IntoIterator for Bytes {
839839
type IntoIter = BytesIter;
840840

841841
fn into_iter(self) -> Self::IntoIter {
842-
BytesIter(self)
842+
BytesIter::new(self)
843843
}
844844
}
845845

846846
#[derive(Clone)]
847-
pub struct BytesIter(Bytes);
847+
pub struct BytesIter {
848+
bin: Bytes,
849+
start: u32, // inclusive
850+
end: u32, // exclusive
851+
}
848852

849853
impl BytesIter {
850-
fn into_bin(self) -> Bytes {
851-
self.0
854+
fn new(bin: Bytes) -> Self {
855+
Self {
856+
start: 0,
857+
end: bin.len(),
858+
bin,
859+
}
852860
}
853861
}
854862

855863
impl Iterator for BytesIter {
856864
type Item = u8;
857865

858866
fn next(&mut self) -> Option<Self::Item> {
859-
if self.0.is_empty() {
860-
None
867+
if self.start < self.end {
868+
let val = self.bin.get_unchecked(self.start);
869+
self.start += 1;
870+
Some(val)
861871
} else {
862-
let val: u32 = self
863-
.0
864-
.env()
865-
.bytes_front(self.0.obj)
866-
.unwrap_infallible()
867-
.into();
868-
self.0 = self.0.slice(1..);
869-
Some(val as u8)
872+
None
870873
}
871874
}
872875

873876
fn size_hint(&self) -> (usize, Option<usize>) {
874-
let len = self.0.len() as usize;
877+
let len = (self.end - self.start) as usize;
875878
(len, Some(len))
876879
}
877880
}
878881

879882
impl DoubleEndedIterator for BytesIter {
880883
fn next_back(&mut self) -> Option<Self::Item> {
881-
let len = self.0.len();
882-
if len == 0 {
883-
None
884+
if self.start < self.end {
885+
self.end -= 1;
886+
Some(self.bin.get_unchecked(self.end))
884887
} else {
885-
let val: u32 = self
886-
.0
887-
.env()
888-
.bytes_back(self.0.obj)
889-
.unwrap_infallible()
890-
.into();
891-
self.0 = self.0.slice(..len - 1);
892-
Some(val as u8)
888+
None
893889
}
894890
}
895891
}
@@ -898,7 +894,7 @@ impl FusedIterator for BytesIter {}
898894

899895
impl ExactSizeIterator for BytesIter {
900896
fn len(&self) -> usize {
901-
self.0.len() as usize
897+
(self.end - self.start) as usize
902898
}
903899
}
904900

@@ -1304,7 +1300,7 @@ impl<const N: usize> IntoIterator for BytesN<N> {
13041300
type IntoIter = BytesIter;
13051301

13061302
fn into_iter(self) -> Self::IntoIter {
1307-
BytesIter(self.0)
1303+
BytesIter::new(self.0)
13081304
}
13091305
}
13101306

@@ -1532,6 +1528,80 @@ mod test {
15321528
assert_eq!(iter.next_back(), None);
15331529
}
15341530

1531+
#[test]
1532+
fn test_bin_iter_large() {
1533+
let env = Env::default();
1534+
const N: usize = 100;
1535+
let mut data = [0u8; N];
1536+
for i in 0..N {
1537+
data[i] = i as u8;
1538+
}
1539+
let bin = Bytes::from_slice(&env, &data);
1540+
1541+
// Forward yields every byte in order, exactly N of them.
1542+
let mut count = 0usize;
1543+
for (i, b) in bin.iter().enumerate() {
1544+
assert_eq!(b, data[i]);
1545+
count += 1;
1546+
}
1547+
assert_eq!(count, N);
1548+
1549+
// Reverse yields every byte in reverse order.
1550+
let mut expect = N;
1551+
for b in bin.iter().rev() {
1552+
expect -= 1;
1553+
assert_eq!(b, data[expect]);
1554+
}
1555+
assert_eq!(expect, 0);
1556+
1557+
// ExactSizeIterator length tracks remaining across a bulk chunk read.
1558+
let mut it = bin.iter();
1559+
assert_eq!(it.len(), N);
1560+
assert_eq!(it.size_hint(), (N, Some(N)));
1561+
it.next();
1562+
assert_eq!(it.len(), N - 1);
1563+
1564+
// Interleaved double-ended iteration: the front and back cursors must
1565+
// meet correctly in the middle.
1566+
let mut it = bin.iter();
1567+
let mut lo = 0usize;
1568+
let mut hi = N;
1569+
let mut front = true;
1570+
while lo < hi {
1571+
if front {
1572+
assert_eq!(it.next(), Some(data[lo]));
1573+
lo += 1;
1574+
} else {
1575+
assert_eq!(it.next_back(), Some(data[hi - 1]));
1576+
hi -= 1;
1577+
}
1578+
front = !front;
1579+
}
1580+
assert_eq!(it.next(), None);
1581+
assert_eq!(it.next_back(), None);
1582+
assert_eq!(it.len(), 0);
1583+
1584+
for len in [32usize, 33] {
1585+
let mut d = [0u8; 33];
1586+
for i in 0..len {
1587+
d[i] = (200 - i) as u8;
1588+
}
1589+
let b = Bytes::from_slice(&env, &d[..len]);
1590+
let mut i = 0;
1591+
for x in b.iter() {
1592+
assert_eq!(x, d[i]);
1593+
i += 1;
1594+
}
1595+
assert_eq!(i, len);
1596+
let mut j = len;
1597+
for x in b.iter().rev() {
1598+
j -= 1;
1599+
assert_eq!(x, d[j]);
1600+
}
1601+
assert_eq!(j, 0);
1602+
}
1603+
}
1604+
15351605
#[test]
15361606
fn test_array_binary_borrow() {
15371607
fn get_len(b: impl Borrow<Bytes>) -> u32 {

0 commit comments

Comments
 (0)