Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various serialization simplifcations and optimizations #9039

Merged
merged 9 commits into from Nov 9, 2016
Merged

Various serialization simplifcations and optimizations #9039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
50e8a9c
Remove unused ReadVersion and WriteVersion
sipa Oct 28, 2016
c2c5d42
Make streams' read and write return void
sipa Oct 29, 2016
fad9b66
Make nType and nVersion private and sometimes const
sipa Oct 28, 2016
657e05a
Make GetSerializeSize a wrapper on top of CSizeComputer
sipa Oct 28, 2016
5284721
Get rid of nType and nVersion
sipa Oct 28, 2016
a603925
Avoid -Wshadow errors
sipa Oct 30, 2016
a2929a2
Make CSerAction's ForRead() constexpr
sipa Oct 29, 2016
25a211a
Add optimized CSizeComputer serializers
sipa Oct 29, 2016
d59a518
Use fixed preallocation instead of costly GetSerializeSize
sipa Oct 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/addrdb.h
View file
Open in desktop
Expand Up @@ -46,7 +46,7 @@ class CBanEntry
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(this->nVersion);
READWRITE(nCreateTime);
READWRITE(nBanUntil);
Expand Down
11 changes: 3 additions & 8 deletions src/addrman.h
View file
Open in desktop
Expand Up @@ -58,7 +58,7 @@ class CAddrInfo : public CAddress
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(*(CAddress*)this);
READWRITE(source);
READWRITE(nLastSuccess);
Expand Down Expand Up @@ -293,7 +293,7 @@ class CAddrMan
* very little in common.
*/
template<typename Stream>
void Serialize(Stream &s, int nType, int nVersionDummy) const
void Serialize(Stream &s) const
{
LOCK(cs);

Expand Down Expand Up @@ -343,7 +343,7 @@ class CAddrMan
}

template<typename Stream>
void Unserialize(Stream& s, int nType, int nVersionDummy)
void Unserialize(Stream& s)
{
LOCK(cs);

Expand Down Expand Up @@ -448,11 +448,6 @@ class CAddrMan
Check();
}

unsigned int GetSerializeSize(int nType, int nVersion) const
{
return (CSizeComputer(nType, nVersion) << *this).size();
}

void Clear()
{
std::vector<int>().swap(vRandom);
Expand Down
2 changes: 1 addition & 1 deletion src/amount.h
View file
Open in desktop
Expand Up @@ -64,7 +64,7 @@ class CFeeRate
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(nSatoshisPerK);
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/blockencodings.cpp
View file
Open in desktop
Expand Up @@ -131,7 +131,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
break;
}

LogPrint("cmpctblock", "Initialized PartiallyDownloadedBlock for block %s using a cmpctblock of size %lu\n", cmpctblock.header.GetHash().ToString(), cmpctblock.GetSerializeSize(SER_NETWORK, PROTOCOL_VERSION));
LogPrint("cmpctblock", "Initialized PartiallyDownloadedBlock for block %s using a cmpctblock of size %lu\n", cmpctblock.header.GetHash().ToString(), GetSerializeSize(cmpctblock, SER_NETWORK, PROTOCOL_VERSION));

return READ_STATUS_OK;
}
Expand Down
10 changes: 5 additions & 5 deletions src/blockencodings.h
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ struct TransactionCompressor {
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(tx); //TODO: Compress tx encoding
}
};
Expand All @@ -35,7 +35,7 @@ class BlockTransactionsRequest {
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(blockhash);
uint64_t indexes_size = (uint64_t)indexes.size();
READWRITE(COMPACTSIZE(indexes_size));
Expand Down Expand Up @@ -81,7 +81,7 @@ class BlockTransactions {
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(blockhash);
uint64_t txn_size = (uint64_t)txn.size();
READWRITE(COMPACTSIZE(txn_size));
Expand Down Expand Up @@ -109,7 +109,7 @@ struct PrefilledTransaction {
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
uint64_t idx = index;
READWRITE(COMPACTSIZE(idx));
if (idx > std::numeric_limits<uint16_t>::max())
Expand Down Expand Up @@ -155,7 +155,7 @@ class CBlockHeaderAndShortTxIDs {
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(header);
READWRITE(nonce);

Expand Down
2 changes: 1 addition & 1 deletion src/bloom.h
View file
Open in desktop
Expand Up @@ -73,7 +73,7 @@ class CBloomFilter
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(vData);
READWRITE(nHashFuncs);
READWRITE(nTweak);
Expand Down
9 changes: 5 additions & 4 deletions src/chain.h
View file
Open in desktop
Expand Up @@ -28,7 +28,7 @@ class CBlockFileInfo
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(VARINT(nBlocks));
READWRITE(VARINT(nSize));
READWRITE(VARINT(nUndoSize));
Expand Down Expand Up @@ -76,7 +76,7 @@ struct CDiskBlockPos
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(VARINT(nFile));
READWRITE(VARINT(nPos));
}
Expand Down Expand Up @@ -357,8 +357,9 @@ class CDiskBlockIndex : public CBlockIndex
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
if (!(nType & SER_GETHASH))
inline void SerializationOp(Stream& s, Operation ser_action) {
int nVersion = s.GetVersion();
if (!(s.GetType() & SER_GETHASH))
READWRITE(VARINT(nVersion));

READWRITE(VARINT(nHeight));
Expand Down
47 changes: 12 additions & 35 deletions src/coins.h
View file
Open in desktop
Expand Up @@ -153,65 +153,42 @@ class CCoins
return fCoinBase;
}

unsigned int GetSerializeSize(int nType, int nVersion) const {
unsigned int nSize = 0;
unsigned int nMaskSize = 0, nMaskCode = 0;
CalcMaskSize(nMaskSize, nMaskCode);
bool fFirst = vout.size() > 0 && !vout[0].IsNull();
bool fSecond = vout.size() > 1 && !vout[1].IsNull();
assert(fFirst || fSecond || nMaskCode);
unsigned int nCode = 8*(nMaskCode - (fFirst || fSecond ? 0 : 1)) + (fCoinBase ? 1 : 0) + (fFirst ? 2 : 0) + (fSecond ? 4 : 0);
// version
nSize += ::GetSerializeSize(VARINT(this->nVersion), nType, nVersion);
// size of header code
nSize += ::GetSerializeSize(VARINT(nCode), nType, nVersion);
// spentness bitmask
nSize += nMaskSize;
// txouts themself
for (unsigned int i = 0; i < vout.size(); i++)
if (!vout[i].IsNull())
nSize += ::GetSerializeSize(CTxOutCompressor(REF(vout[i])), nType, nVersion);
// height
nSize += ::GetSerializeSize(VARINT(nHeight), nType, nVersion);
return nSize;
}

template<typename Stream>
void Serialize(Stream &s, int nType, int nVersion) const {
void Serialize(Stream &s) const {
unsigned int nMaskSize = 0, nMaskCode = 0;
CalcMaskSize(nMaskSize, nMaskCode);
bool fFirst = vout.size() > 0 && !vout[0].IsNull();
bool fSecond = vout.size() > 1 && !vout[1].IsNull();
assert(fFirst || fSecond || nMaskCode);
unsigned int nCode = 8*(nMaskCode - (fFirst || fSecond ? 0 : 1)) + (fCoinBase ? 1 : 0) + (fFirst ? 2 : 0) + (fSecond ? 4 : 0);
// version
::Serialize(s, VARINT(this->nVersion), nType, nVersion);
::Serialize(s, VARINT(this->nVersion));
// header code
::Serialize(s, VARINT(nCode), nType, nVersion);
::Serialize(s, VARINT(nCode));
// spentness bitmask
for (unsigned int b = 0; b<nMaskSize; b++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking this is also a performance regression...do we care about the time taken to iterate over the nMastkSize bits (ie do we ever care about the performance of CCoins serialization that much?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think this will be optimized out. It's a loop with known count in CCoin::Serialize, where each iteration results in a counter increment by one. Also, yes, I don't think we care.

unsigned char chAvail = 0;
for (unsigned int i = 0; i < 8 && 2+b*8+i < vout.size(); i++)
if (!vout[2+b*8+i].IsNull())
chAvail |= (1 << i);
::Serialize(s, chAvail, nType, nVersion);
::Serialize(s, chAvail);
}
// txouts themself
for (unsigned int i = 0; i < vout.size(); i++) {
if (!vout[i].IsNull())
::Serialize(s, CTxOutCompressor(REF(vout[i])), nType, nVersion);
::Serialize(s, CTxOutCompressor(REF(vout[i])));
}
// coinbase height
::Serialize(s, VARINT(nHeight), nType, nVersion);
::Serialize(s, VARINT(nHeight));
}

template<typename Stream>
void Unserialize(Stream &s, int nType, int nVersion) {
void Unserialize(Stream &s) {
unsigned int nCode = 0;
// version
::Unserialize(s, VARINT(this->nVersion), nType, nVersion);
::Unserialize(s, VARINT(this->nVersion));
// header code
::Unserialize(s, VARINT(nCode), nType, nVersion);
::Unserialize(s, VARINT(nCode));
fCoinBase = nCode & 1;
std::vector<bool> vAvail(2, false);
vAvail[0] = (nCode & 2) != 0;
Expand All @@ -220,7 +197,7 @@ class CCoins
// spentness bitmask
while (nMaskCode > 0) {
unsigned char chAvail = 0;
::Unserialize(s, chAvail, nType, nVersion);
::Unserialize(s, chAvail);
for (unsigned int p = 0; p < 8; p++) {
bool f = (chAvail & (1 << p)) != 0;
vAvail.push_back(f);
Expand All @@ -232,10 +209,10 @@ class CCoins
vout.assign(vAvail.size(), CTxOut());
for (unsigned int i = 0; i < vAvail.size(); i++) {
if (vAvail[i])
::Unserialize(s, REF(CTxOutCompressor(vout[i])), nType, nVersion);
::Unserialize(s, REF(CTxOutCompressor(vout[i])));
}
// coinbase height
::Unserialize(s, VARINT(nHeight), nType, nVersion);
::Unserialize(s, VARINT(nHeight));
Cleanup();
}

Expand Down
14 changes: 3 additions & 11 deletions src/compressor.h
View file
Open in desktop
Expand Up @@ -55,16 +55,8 @@ class CScriptCompressor
public:
CScriptCompressor(CScript &scriptIn) : script(scriptIn) { }

unsigned int GetSerializeSize(int nType, int nVersion) const {
std::vector<unsigned char> compr;
if (Compress(compr))
return compr.size();
unsigned int nSize = script.size() + nSpecialScripts;
return script.size() + VARINT(nSize).GetSerializeSize(nType, nVersion);
}

template<typename Stream>
void Serialize(Stream &s, int nType, int nVersion) const {
void Serialize(Stream &s) const {
std::vector<unsigned char> compr;
if (Compress(compr)) {
s << CFlatData(compr);
Expand All @@ -76,7 +68,7 @@ class CScriptCompressor
}

template<typename Stream>
void Unserialize(Stream &s, int nType, int nVersion) {
void Unserialize(Stream &s) {
unsigned int nSize = 0;
s >> VARINT(nSize);
if (nSize < nSpecialScripts) {
Expand Down Expand Up @@ -112,7 +104,7 @@ class CTxOutCompressor
ADD_SERIALIZE_METHODS;

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
inline void SerializationOp(Stream& s, Operation ser_action) {
if (!ser_action.ForRead()) {
uint64_t nVal = CompressAmount(txout.nValue);
READWRITE(VARINT(nVal));
Expand Down
15 changes: 9 additions & 6 deletions src/dbwrapper.h
View file
Open in desktop
Expand Up @@ -17,6 +17,9 @@
#include <leveldb/db.h>
#include <leveldb/write_batch.h>

static const size_t DBWRAPPER_PREALLOC_KEY_SIZE = 64;
static const size_t DBWRAPPER_PREALLOC_VALUE_SIZE = 1024;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no reason not to go higher here? Keys should ~always be 35 bytes (ish, but 64 is fine), but values could be higher...but might as well also prealloc more than 1K for values since they're occasionally higher and the memory usage isnt an issue. Did you benchmark this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did benchmark the change overall, and the "Flush to disk" time went down by around 5% from just this. I did not benchmark any other sizes, but my expectation is that as long as the key size is over 35, and the value over 100 or so, there will not be any measurable difference, as these cases occur so infrequently, and a vector resize isn't the end of the world.


class dbwrapper_error : public std::runtime_error
{
public:
Expand Down Expand Up @@ -60,12 +63,12 @@ class CDBBatch
void Write(const K& key, const V& value)
{
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(ssKey.GetSerializeSize(key));
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
leveldb::Slice slKey(&ssKey[0], ssKey.size());

CDataStream ssValue(SER_DISK, CLIENT_VERSION);
ssValue.reserve(ssValue.GetSerializeSize(value));
ssValue.reserve(DBWRAPPER_PREALLOC_VALUE_SIZE);
ssValue << value;
ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent));
leveldb::Slice slValue(&ssValue[0], ssValue.size());
Expand All @@ -77,7 +80,7 @@ class CDBBatch
void Erase(const K& key)
{
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(ssKey.GetSerializeSize(key));
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
leveldb::Slice slKey(&ssKey[0], ssKey.size());

Expand Down Expand Up @@ -107,7 +110,7 @@ class CDBIterator

template<typename K> void Seek(const K& key) {
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(ssKey.GetSerializeSize(key));
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
leveldb::Slice slKey(&ssKey[0], ssKey.size());
piter->Seek(slKey);
Expand Down Expand Up @@ -200,7 +203,7 @@ class CDBWrapper
bool Read(const K& key, V& value) const
{
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(ssKey.GetSerializeSize(key));
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
leveldb::Slice slKey(&ssKey[0], ssKey.size());

Expand Down Expand Up @@ -234,7 +237,7 @@ class CDBWrapper
bool Exists(const K& key) const
{
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(ssKey.GetSerializeSize(key));
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
leveldb::Slice slKey(&ssKey[0], ssKey.size());

Expand Down
12 changes: 7 additions & 5 deletions src/hash.h
View file
Open in desktop
Expand Up @@ -132,15 +132,17 @@ class CHashWriter
private:
CHash256 ctx;

const int nType;
const int nVersion;
public:
int nType;
int nVersion;

CHashWriter(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {}

CHashWriter& write(const char *pch, size_t size) {
int GetType() const { return nType; }
int GetVersion() const { return nVersion; }

void write(const char *pch, size_t size) {
ctx.Write((const unsigned char*)pch, size);
return (*this);
}

// invalidates the object
Expand All @@ -153,7 +155,7 @@ class CHashWriter
template<typename T>
CHashWriter& operator<<(const T& obj) {
// Serialize to this stream
::Serialize(*this, obj, nType, nVersion);
::Serialize(*this, obj);
return (*this);
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/key.h
View file
Open in desktop
Expand Up @@ -162,7 +162,7 @@ struct CExtKey {
CExtPubKey Neuter() const;
void SetMaster(const unsigned char* seed, unsigned int nSeedLen);
template <typename Stream>
void Serialize(Stream& s, int nType, int nVersion) const
void Serialize(Stream& s) const
{
unsigned int len = BIP32_EXTKEY_SIZE;
::WriteCompactSize(s, len);
Expand All @@ -171,7 +171,7 @@ struct CExtKey {
s.write((const char *)&code[0], len);
}
template <typename Stream>
void Unserialize(Stream& s, int nType, int nVersion)
void Unserialize(Stream& s)
{
unsigned int len = ::ReadCompactSize(s);
unsigned char code[BIP32_EXTKEY_SIZE];
Expand Down