Skip to content

Commit 9c754ca

Browse files
dpcdpc
committed
Take Writer/Reader by &mut in consensus en/decoding
Fix #1020 (see more relevant discussion there) This definitely makes the amount of generics compiler has to generate by avoding generating the same functions for `R`, &mut R`, `&mut &mut R` and so on. old: ``` > ls -al target/release/deps/bitcoin-07a9dabf1f3e0266 -rwxrwxr-x 1 dpc dpc 9947832 Jun 2 22:42 target/release/deps/bitcoin-07a9dabf1f3e0266 > strip target/release/deps/bitcoin-07a9dabf1f3e0266 > ls -al target/release/deps/bitcoin-07a9dabf1f3e0266 -rwxrwxr-x 1 dpc dpc 4463024 Jun 2 22:46 target/release/deps/bitcoin-07a9dabf1f3e0266 ``` new: ``` > ls -al target/release/deps/bitcoin-07a9dabf1f3e0266 -rwxrwxr-x 1 dpc dpc 9866800 Jun 2 22:44 target/release/deps/bitcoin-07a9dabf1f3e0266 > strip target/release/deps/bitcoin-07a9dabf1f3e0266 > ls -al target/release/deps/bitcoin-07a9dabf1f3e0266 -rwxrwxr-x 1 dpc dpc 4393392 Jun 2 22:45 target/release/deps/bitcoin-07a9dabf1f3e0266 ``` In the unit-test binary itself, it saves ~100KB of data. I did not expect much performance gains, but turn out I was wrong(*): old: ``` test blockdata::block::benches::bench_block_deserialize ... bench: 1,072,710 ns/iter (+/- 21,871) test blockdata::block::benches::bench_block_serialize ... bench: 191,223 ns/iter (+/- 5,833) test blockdata::block::benches::bench_block_serialize_logic ... bench: 37,543 ns/iter (+/- 732) test blockdata::block::benches::bench_stream_reader ... bench: 1,872,455 ns/iter (+/- 149,519) test blockdata::transaction::benches::bench_transaction_deserialize ... bench: 136 ns/iter (+/- 3) test blockdata::transaction::benches::bench_transaction_serialize ... bench: 51 ns/iter (+/- 8) test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench: 5 ns/iter (+/- 0) test blockdata::transaction::benches::bench_transaction_size ... bench: 3 ns/iter (+/- 0) ``` new: ``` test blockdata::block::benches::bench_block_deserialize ... bench: 1,028,574 ns/iter (+/- 10,910) test blockdata::block::benches::bench_block_serialize ... bench: 162,143 ns/iter (+/- 3,363) test blockdata::block::benches::bench_block_serialize_logic ... bench: 30,725 ns/iter (+/- 695) test blockdata::block::benches::bench_stream_reader ... bench: 1,437,071 ns/iter (+/- 53,694) test blockdata::transaction::benches::bench_transaction_deserialize ... bench: 92 ns/iter (+/- 2) test blockdata::transaction::benches::bench_transaction_serialize ... bench: 17 ns/iter (+/- 0) test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench: 5 ns/iter (+/- 0) test blockdata::transaction::benches::bench_transaction_size ... bench: 4 ns/iter (+/- 0) ``` (*) - I'm benchmarking on a noisy laptop. Take this with a grain of salt. But I think at least it doesn't make anything slower. While doing all this manual labor that will probably generate conflicts, I took a liberty of changing generic type names and variable names to `r` and `R` (reader) and `w` and `W` for writer.
1 parent cf5503a commit 9c754ca

20 files changed

Lines changed: 330 additions & 321 deletions

‎src/blockdata/script.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use crate::consensus::encode::MAX_VEC_SIZE;
2727
use crate::prelude::*;
2828

2929
use crate::io;
30+
use io::Read as _;
3031
use core::{fmt, default::Default};
3132
use core::ops::Index;
3233

@@ -1092,20 +1093,20 @@ impl serde::Serialize for Script {
10921093

10931094
impl Encodable for Script {
10941095
#[inline]
1095-
fn consensus_encode<S: io::Write>(&self, s: S) -> Result<usize, io::Error> {
1096-
self.0.consensus_encode(s)
1096+
fn consensus_encode<W: io::Write>(&self, w: &mut W) -> Result<usize, io::Error> {
1097+
self.0.consensus_encode(w)
10971098
}
10981099
}
10991100

11001101
impl Decodable for Script {
11011102
#[inline]
1102-
fn consensus_decode_from_finite_reader<D: io::Read>(d: D) -> Result<Self, encode::Error> {
1103-
Ok(Script(Decodable::consensus_decode_from_finite_reader(d)?))
1103+
fn consensus_decode_from_finite_reader<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
1104+
Ok(Script(Decodable::consensus_decode_from_finite_reader(r)?))
11041105
}
11051106

11061107
#[inline]
1107-
fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
1108-
Self::consensus_decode_from_finite_reader(d.take(MAX_VEC_SIZE as u64))
1108+
fn consensus_decode<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
1109+
Self::consensus_decode_from_finite_reader(r.take(MAX_VEC_SIZE as u64).by_ref())
11091110
}
11101111
}
11111112

‎src/blockdata/transaction.rs‎

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use crate::prelude::*;
2727

2828
use crate::io;
29+
use io::Read as _;
2930
use core::{fmt, str, default::Default};
3031

3132
use crate::hashes::{self, Hash, sha256d};
@@ -653,50 +654,50 @@ impl Transaction {
653654
impl_consensus_encoding!(TxOut, value, script_pubkey);
654655

655656
impl Encodable for OutPoint {
656-
fn consensus_encode<S: io::Write>(&self, mut s: S) -> Result<usize, io::Error> {
657-
let len = self.txid.consensus_encode(&mut s)?;
658-
Ok(len + self.vout.consensus_encode(s)?)
657+
fn consensus_encode<W: io::Write>(&self, w: &mut W) -> Result<usize, io::Error> {
658+
let len = self.txid.consensus_encode(w)?;
659+
Ok(len + self.vout.consensus_encode(w)?)
659660
}
660661
}
661662
impl Decodable for OutPoint {
662-
fn consensus_decode<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
663+
fn consensus_decode<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
663664
Ok(OutPoint {
664-
txid: Decodable::consensus_decode(&mut d)?,
665-
vout: Decodable::consensus_decode(d)?,
665+
txid: Decodable::consensus_decode(r)?,
666+
vout: Decodable::consensus_decode(r)?,
666667
})
667668
}
668669
}
669670

670671
impl Encodable for TxIn {
671-
fn consensus_encode<S: io::Write>(&self, mut s: S) -> Result<usize, io::Error> {
672+
fn consensus_encode<W: io::Write>(&self, w: &mut W) -> Result<usize, io::Error> {
672673
let mut len = 0;
673-
len += self.previous_output.consensus_encode(&mut s)?;
674-
len += self.script_sig.consensus_encode(&mut s)?;
675-
len += self.sequence.consensus_encode(s)?;
674+
len += self.previous_output.consensus_encode(w)?;
675+
len += self.script_sig.consensus_encode(w)?;
676+
len += self.sequence.consensus_encode(w)?;
676677
Ok(len)
677678
}
678679
}
679680
impl Decodable for TxIn {
680681
#[inline]
681-
fn consensus_decode_from_finite_reader<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
682+
fn consensus_decode_from_finite_reader<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
682683
Ok(TxIn {
683-
previous_output: Decodable::consensus_decode_from_finite_reader(&mut d)?,
684-
script_sig: Decodable::consensus_decode_from_finite_reader(&mut d)?,
685-
sequence: Decodable::consensus_decode_from_finite_reader(d)?,
684+
previous_output: Decodable::consensus_decode_from_finite_reader(r)?,
685+
script_sig: Decodable::consensus_decode_from_finite_reader(r)?,
686+
sequence: Decodable::consensus_decode_from_finite_reader(r)?,
686687
witness: Witness::default(),
687688
})
688689
}
689690

690691
#[inline]
691-
fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
692-
Self::consensus_decode_from_finite_reader(d.take(MAX_VEC_SIZE as u64))
692+
fn consensus_decode<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
693+
Self::consensus_decode_from_finite_reader(r.take(MAX_VEC_SIZE as u64).by_ref())
693694
}
694695
}
695696

696697
impl Encodable for Transaction {
697-
fn consensus_encode<S: io::Write>(&self, mut s: S) -> Result<usize, io::Error> {
698+
fn consensus_encode<W: io::Write>(&self, w: &mut W) -> Result<usize, io::Error> {
698699
let mut len = 0;
699-
len += self.version.consensus_encode(&mut s)?;
700+
len += self.version.consensus_encode(w)?;
700701
// To avoid serialization ambiguity, no inputs means we use BIP141 serialization (see
701702
// `Transaction` docs for full explanation).
702703
let mut have_witness = self.input.is_empty();
@@ -707,36 +708,36 @@ impl Encodable for Transaction {
707708
}
708709
}
709710
if !have_witness {
710-
len += self.input.consensus_encode(&mut s)?;
711-
len += self.output.consensus_encode(&mut s)?;
711+
len += self.input.consensus_encode(w)?;
712+
len += self.output.consensus_encode(w)?;
712713
} else {
713-
len += 0u8.consensus_encode(&mut s)?;
714-
len += 1u8.consensus_encode(&mut s)?;
715-
len += self.input.consensus_encode(&mut s)?;
716-
len += self.output.consensus_encode(&mut s)?;
714+
len += 0u8.consensus_encode(w)?;
715+
len += 1u8.consensus_encode(w)?;
716+
len += self.input.consensus_encode(w)?;
717+
len += self.output.consensus_encode(w)?;
717718
for input in &self.input {
718-
len += input.witness.consensus_encode(&mut s)?;
719+
len += input.witness.consensus_encode(w)?;
719720
}
720721
}
721-
len += self.lock_time.consensus_encode(s)?;
722+
len += self.lock_time.consensus_encode(w)?;
722723
Ok(len)
723724
}
724725
}
725726

726727
impl Decodable for Transaction {
727-
fn consensus_decode_from_finite_reader<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
728-
let version = i32::consensus_decode_from_finite_reader(&mut d)?;
729-
let input = Vec::<TxIn>::consensus_decode_from_finite_reader(&mut d)?;
728+
fn consensus_decode_from_finite_reader<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
729+
let version = i32::consensus_decode_from_finite_reader(r)?;
730+
let input = Vec::<TxIn>::consensus_decode_from_finite_reader(r)?;
730731
// segwit
731732
if input.is_empty() {
732-
let segwit_flag = u8::consensus_decode_from_finite_reader(&mut d)?;
733+
let segwit_flag = u8::consensus_decode_from_finite_reader(r)?;
733734
match segwit_flag {
734735
// BIP144 input witnesses
735736
1 => {
736-
let mut input = Vec::<TxIn>::consensus_decode_from_finite_reader(&mut d)?;
737-
let output = Vec::<TxOut>::consensus_decode_from_finite_reader(&mut d)?;
737+
let mut input = Vec::<TxIn>::consensus_decode_from_finite_reader(r)?;
738+
let output = Vec::<TxOut>::consensus_decode_from_finite_reader(r)?;
738739
for txin in input.iter_mut() {
739-
txin.witness = Decodable::consensus_decode_from_finite_reader(&mut d)?;
740+
txin.witness = Decodable::consensus_decode_from_finite_reader(r)?;
740741
}
741742
if !input.is_empty() && input.iter().all(|input| input.witness.is_empty()) {
742743
Err(encode::Error::ParseFailed("witness flag set but no witnesses present"))
@@ -745,7 +746,7 @@ impl Decodable for Transaction {
745746
version,
746747
input,
747748
output,
748-
lock_time: Decodable::consensus_decode_from_finite_reader(d)?,
749+
lock_time: Decodable::consensus_decode_from_finite_reader(r)?,
749750
})
750751
}
751752
}
@@ -757,14 +758,14 @@ impl Decodable for Transaction {
757758
Ok(Transaction {
758759
version,
759760
input,
760-
output: Decodable::consensus_decode_from_finite_reader(&mut d)?,
761-
lock_time: Decodable::consensus_decode_from_finite_reader(d)?,
761+
output: Decodable::consensus_decode_from_finite_reader(r)?,
762+
lock_time: Decodable::consensus_decode_from_finite_reader(r)?,
762763
})
763764
}
764765
}
765766

766-
fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
767-
Self::consensus_decode_from_finite_reader(d.take(MAX_VEC_SIZE as u64))
767+
fn consensus_decode<R: io::Read>(r: &mut R) -> Result<Self, encode::Error> {
768+
Self::consensus_decode_from_finite_reader(&mut r.take(MAX_VEC_SIZE as u64))
768769
}
769770
}
770771

‎src/blockdata/witness.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ pub struct Iter<'a> {
4949
}
5050

5151
impl Decodable for Witness {
52-
fn consensus_decode<D: Read>(mut d: D) -> Result<Self, Error> {
53-
let witness_elements = VarInt::consensus_decode(&mut d)?.0 as usize;
52+
fn consensus_decode<R: Read>(r: &mut R) -> Result<Self, Error> {
53+
let witness_elements = VarInt::consensus_decode(r)?.0 as usize;
5454
if witness_elements == 0 {
5555
Ok(Witness::default())
5656
} else {
@@ -65,7 +65,7 @@ impl Decodable for Witness {
6565
for _ in 0..witness_elements {
6666
second_to_last = last;
6767
last = cursor;
68-
let element_size_varint = VarInt::consensus_decode(&mut d)?;
68+
let element_size_varint = VarInt::consensus_decode(r)?;
6969
let element_size_varint_len = element_size_varint.len();
7070
let element_size = element_size_varint.0 as usize;
7171
let required_len = cursor
@@ -89,9 +89,9 @@ impl Decodable for Witness {
8989

9090
resize_if_needed(&mut content, required_len);
9191
element_size_varint
92-
.consensus_encode(&mut content[cursor..cursor + element_size_varint_len])?;
92+
.consensus_encode(&mut &mut content[cursor..cursor + element_size_varint_len])?;
9393
cursor += element_size_varint_len;
94-
d.read_exact(&mut content[cursor..cursor + element_size])?;
94+
r.read_exact(&mut content[cursor..cursor + element_size])?;
9595
cursor += element_size;
9696
}
9797
content.truncate(cursor);
@@ -116,10 +116,10 @@ fn resize_if_needed(vec: &mut Vec<u8>, required_len: usize) {
116116
}
117117

118118
impl Encodable for Witness {
119-
fn consensus_encode<W: Write>(&self, mut writer: W) -> Result<usize, io::Error> {
119+
fn consensus_encode<W: Write>(&self, w: &mut W) -> Result<usize, io::Error> {
120120
let len = VarInt(self.witness_elements as u64);
121-
len.consensus_encode(&mut writer)?;
122-
writer.emit_slice(&self.content[..])?;
121+
len.consensus_encode(w)?;
122+
w.emit_slice(&self.content[..])?;
123123
Ok(self.content.len() + len.len())
124124
}
125125
}
@@ -148,7 +148,7 @@ impl Witness {
148148
last = cursor;
149149
let el_len_varint = VarInt(el.len() as u64);
150150
el_len_varint
151-
.consensus_encode(&mut content[cursor..cursor + el_len_varint.len()])
151+
.consensus_encode(&mut &mut content[cursor..cursor + el_len_varint.len()])
152152
.expect("writers on vec don't errors, space granted by content_size");
153153
cursor += el_len_varint.len();
154154
content[cursor..cursor + el.len()].copy_from_slice(&el);
@@ -211,7 +211,7 @@ impl Witness {
211211
.resize(current_content_len + element_len_varint.len() + new_element.len(), 0);
212212
let end_varint = current_content_len + element_len_varint.len();
213213
element_len_varint
214-
.consensus_encode(&mut self.content[current_content_len..end_varint])
214+
.consensus_encode(&mut &mut self.content[current_content_len..end_varint])
215215
.expect("writers on vec don't error, space granted through previous resize");
216216
self.content[end_varint..].copy_from_slice(new_element);
217217
}
@@ -228,7 +228,7 @@ impl Witness {
228228

229229

230230
fn element_at(&self, index: usize) -> Option<&[u8]> {
231-
let varint = VarInt::consensus_decode(&self.content[index..]).ok()?;
231+
let varint = VarInt::consensus_decode(&mut &self.content[index..]).ok()?;
232232
let start = index + varint.len();
233233
Some(&self.content[start..start + varint.0 as usize])
234234
}
@@ -256,7 +256,7 @@ impl<'a> Iterator for Iter<'a> {
256256
type Item = &'a [u8];
257257

258258
fn next(&mut self) -> Option<Self::Item> {
259-
let varint = VarInt::consensus_decode(self.inner.as_slice()).ok()?;
259+
let varint = VarInt::consensus_decode(&mut self.inner.as_slice()).ok()?;
260260
self.inner.nth(varint.len() - 1)?; // VarInt::len returns at least 1
261261
let len = varint.0 as usize;
262262
let slice = &self.inner.as_slice()[..len];

0 commit comments

Comments
 (0)