Avoid creating a temporary vector for size-prefixed elements #13719

pull sipa wants to merge 1 commits into bitcoin:master from sipa:201807_psbt_no_vec_serialize changing 2 files +16 −10

sipa commented at 9:01 PM on July 19, 2018: member

This is a simple improvement to the PSBT serialization code, avoiding the need for temporary vectors everywhere.
in src/script/sign.h:124 in 0f62cbd252 outdated
```
 122 |  template<typename Stream, typename... X>
 123 |  void SerializeToVector(Stream& s, const X&... args)
 124 |  {
 125 | -    std::vector<unsigned char> ret;
 126 | -    CVectorWriter ss(SER_NETWORK, PROTOCOL_VERSION, ret, 0);
 127 | +    CSizeComputer ss(s.GetType(), s.GetVersion());
```
theuni commented at 9:21 PM on July 19, 2018:

I think this is useful enough to add to add as a function in serialize.h. Making GetSerializeSize() variadic makes sense, but the params would have to be changed around and that means touching a bunch of code.

GetSerializeSizeMany() maybe?

sipa commented at 10:20 PM on July 19, 2018:

Done.
sipa force-pushed on Jul 19, 2018
fanquake added the label Refactoring on Jul 19, 2018
in src/script/sign.h:133 in f6c261f9d2 outdated
```
 138 | -    s >> data;
 139 | -    CDataStream ss(data, SER_NETWORK, PROTOCOL_VERSION);
 140 | -    UnserializeMany(ss, args...);
 141 | -    if (!ss.eof()) {
 142 | +    size_t size = ReadCompactSize(s);
 143 | +    size_t pos = s.size();
```
achow101 commented at 11:13 PM on July 19, 2018:

nit: the use of pos and newpos here is slightly confusing as they do not refer to the positions of anything but rather the remaining number of bytes in the stream.

sipa commented at 11:18 PM on July 19, 2018:

Fixed.
achow101 commented at 11:13 PM on July 19, 2018: member

~utACK f6c261f9d24d1c196e559d2faf33b38e37e98be3~

utACK 84547fa6d408bdda1685f6d5972232bb19d97a7d
Avoid creating a temporary vector for size-prefixed elements 84547fa6d4
sipa force-pushed on Jul 19, 2018
laanwj commented at 1:46 PM on July 20, 2018: member

utACK 84547fa6d408bdda1685f6d5972232bb19d97a7d
laanwj merged this on Jul 23, 2018
laanwj closed this on Jul 23, 2018
laanwj referenced this in commit c0a47da725 on Jul 23, 2018
DrahtBot locked this on Sep 8, 2021

Contributors

Labels