Skip to content

Commit

Permalink
refactor: re-order headers and forward declarations to improve compil…
Browse files Browse the repository at this point in the history
…e time (#5693)

## Issue being fixed or feature implemented
Some headers include other heavy headers, such as `logging.h`,
`tinyformat.h`, `iostream`. These headers are heavy and increase
compilation time on scale of whole project drastically because can be
used in many other headers.

## What was done?
Moved many heavy includes from headers to cpp files to optimize
compilation time.
In some places  added forward declarations if it is reasonable.

As side effect removed 2 circular dependencies:
```
"llmq/debug -> llmq/dkgsessionhandler -> llmq/debug"
"llmq/debug -> llmq/dkgsessionhandler -> llmq/dkgsession -> llmq/debug"
```


## How Has This Been Tested?
Run build 2 times before refactoring and after refactoring: `make clean
&& sleep 10s; time make -j18`

Before refactoring:
```
real    5m37,826s
user    77m12,075s
sys     6m20,547s

real    5m32,626s
user    76m51,143s
sys     6m24,511s
```

After refactoring:
```
real    5m18,509s
user    73m32,133s
sys     6m21,590s

real    5m14,466s
user    73m20,942s
sys     6m17,868s
```

~5% of improvement for compilation time. That's not huge, but that's
worth to get merged

There're several more refactorings TODO but better to do them later by
backports:
 - bitcoin/bitcoin#27636
 - bitcoin/bitcoin#26286
 - bitcoin/bitcoin#27238
 - and maybe this one: bitcoin/bitcoin#28200


## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
  • Loading branch information
knst authored Nov 17, 2023
1 parent 7fc8a04 commit ba97f49
Show file tree
Hide file tree
Showing 53 changed files with 237 additions and 157 deletions.
2 changes: 2 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,12 @@ libbitcoin_server_a_SOURCES = \
init.cpp \
governance/governance.cpp \
governance/classes.cpp \
governance/exceptions.cpp \
governance/object.cpp \
governance/validators.cpp \
governance/vote.cpp \
governance/votedb.cpp \
gsl/assert.cpp \
llmq/quorums.cpp \
llmq/blockprocessor.cpp \
llmq/commitment.cpp \
Expand Down
2 changes: 1 addition & 1 deletion src/addrman.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#include <fs.h>
#include <hash.h>
#include <iostream>
#include <ios>
#include <map>
#include <optional>
#include <set>
Expand Down
20 changes: 20 additions & 0 deletions src/chain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,26 @@

#include <chain.h>

#include <tinyformat.h>

std::string CDiskBlockIndex::ToString() const
{
std::string str = "CDiskBlockIndex(";
str += CBlockIndex::ToString();
str += strprintf("\n hashBlock=%s, hashPrev=%s)",
GetBlockHash().ToString(),
hashPrev.ToString());
return str;
}

std::string CBlockIndex::ToString() const
{
return strprintf("CBlockIndex(pprev=%p, nHeight=%d, merkle=%s, hashBlock=%s)",
pprev, nHeight,
hashMerkleRoot.ToString(),
GetBlockHash().ToString());
}

/**
* CChain implementation
*/
Expand Down
19 changes: 2 additions & 17 deletions src/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <consensus/params.h>
#include <flatfile.h>
#include <primitives/block.h>
#include <tinyformat.h>
#include <uint256.h>

#include <vector>
Expand Down Expand Up @@ -281,13 +280,7 @@ class CBlockIndex
return pbegin[(pend - pbegin)/2];
}

std::string ToString() const
{
return strprintf("CBlockIndex(pprev=%p, nHeight=%d, merkle=%s, hashBlock=%s)",
pprev, nHeight,
hashMerkleRoot.ToString(),
GetBlockHash().ToString());
}
std::string ToString() const;

//! Check whether this block index entry is valid up to the passed validity level.
bool IsValid(enum BlockStatus nUpTo = BLOCK_VALID_TRANSACTIONS) const
Expand Down Expand Up @@ -381,16 +374,8 @@ class CDiskBlockIndex : public CBlockIndex
return block.GetHash();
}

std::string ToString() const;

std::string ToString() const
{
std::string str = "CDiskBlockIndex(";
str += CBlockIndex::ToString();
str += strprintf("\n hashBlock=%s, hashPrev=%s)",
GetBlockHash().ToString(),
hashPrev.ToString());
return str;
}
};

/** An in-memory indexed chain of blocks. */
Expand Down
10 changes: 10 additions & 0 deletions src/coinjoin/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <net_processing.h>
#include <netmessagemaker.h>
#include <shutdown.h>
#include <util/check.h>
#include <util/irange.h>
#include <util/moneystr.h>
#include <util/ranges.h>
Expand Down Expand Up @@ -155,6 +156,15 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, PeerManager& peerman, C
}
}

CCoinJoinClientSession::CCoinJoinClientSession(CWallet& pwallet, CJClientManager& clientman, const CMasternodeSync& mn_sync,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_wallet(pwallet),
m_clientman(clientman),
m_manager(*Assert(clientman.Get(pwallet))),
m_mn_sync(mn_sync),
m_queueman(queueman)
{}

void CCoinJoinClientSession::ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv)
{
if (fMasternodeMode) return;
Expand Down
4 changes: 1 addition & 3 deletions src/coinjoin/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <coinjoin/util.h>
#include <coinjoin/coinjoin.h>
#include <util/check.h>
#include <util/translation.h>

#include <atomic>
Expand Down Expand Up @@ -165,8 +164,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession

public:
explicit CCoinJoinClientSession(CWallet& pwallet, CJClientManager& clientman, const CMasternodeSync& mn_sync,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_wallet(pwallet), m_clientman(clientman), m_manager(*Assert(clientman.Get(pwallet))), m_mn_sync(mn_sync), m_queueman(queueman) {}
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman);

void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);

Expand Down
7 changes: 7 additions & 0 deletions src/coinjoin/coinjoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <util/translation.h>
#include <validation.h>

#include <tinyformat.h>
#include <string>

constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10};
Expand Down Expand Up @@ -87,6 +88,12 @@ bool CCoinJoinQueue::IsTimeOutOfBounds(int64_t current_time) const
nTime - current_time > COINJOIN_QUEUE_TIMEOUT;
}

[[nodiscard]] std::string CCoinJoinQueue::ToString() const
{
return strprintf("nDenom=%d, nTime=%lld, fReady=%s, fTried=%s, masternode=%s",
nDenom, nTime, fReady ? "true" : "false", fTried ? "true" : "false", masternodeOutpoint.ToStringShort());
}

uint256 CCoinJoinBroadcastTx::GetSignatureHash() const
{
return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
Expand Down
7 changes: 1 addition & 6 deletions src/coinjoin/coinjoin.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <primitives/transaction.h>
#include <sync.h>
#include <timedata.h>
#include <tinyformat.h>
#include <univalue.h>
#include <util/ranges.h>
#include <util/translation.h>
Expand Down Expand Up @@ -238,11 +237,7 @@ class CCoinJoinQueue
/// Check if a queue is too old or too far into the future
[[nodiscard]] bool IsTimeOutOfBounds(int64_t current_time = GetAdjustedTime()) const;

[[nodiscard]] std::string ToString() const
{
return strprintf("nDenom=%d, nTime=%lld, fReady=%s, fTried=%s, masternode=%s",
nDenom, nTime, fReady ? "true" : "false", fTried ? "true" : "false", masternodeOutpoint.ToStringShort());
}
[[nodiscard]] std::string ToString() const;

friend bool operator==(const CCoinJoinQueue& a, const CCoinJoinQueue& b)
{
Expand Down
1 change: 1 addition & 0 deletions src/consensus/tx_verify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <script/interpreter.h>
#include <consensus/validation.h>
#include <evo/assetlocktx.h>
#include <tinyformat.h>

// TODO remove the following dependencies
#include <chain.h>
Expand Down
12 changes: 6 additions & 6 deletions src/evo/assetlocktx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@
#include <evo/assetlocktx.h>
#include <evo/specialtx.h>

#include <consensus/params.h>

#include <chainparams.h>
#include <logging.h>
#include <validation.h>

#include <llmq/commitment.h>
#include <llmq/signing.h>
#include <llmq/utils.h>
#include <llmq/quorums.h>

#include <chainparams.h>
#include <consensus/params.h>
#include <consensus/validation.h>
#include <logging.h>
#include <tinyformat.h>
#include <util/ranges_set.h>
#include <validation.h>

#include <algorithm>

Expand Down
4 changes: 1 addition & 3 deletions src/evo/assetlocktx.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,17 @@
#define BITCOIN_EVO_ASSETLOCKTX_H

#include <bls/bls_ies.h>
#include <evo/specialtx.h>
#include <primitives/transaction.h>
#include <gsl/pointers.h>

#include <key_io.h>
#include <serialize.h>
#include <tinyformat.h>
#include <univalue.h>

#include <optional>

class CBlockIndex;
class CRangesSet;
class TxValidationState;

class CAssetLockPayload
{
Expand Down
4 changes: 3 additions & 1 deletion src/evo/creditpool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@

#include <evo/assetlocktx.h>
#include <evo/cbtx.h>
#include <evo/specialtx.h>

#include <chain.h>
#include <consensus/validation.h>
#include <llmq/utils.h>
#include <logging.h>
#include <validation.h>
#include <node/blockstorage.h>
#include <validation.h>

#include <algorithm>
#include <exception>
Expand Down
1 change: 1 addition & 0 deletions src/evo/creditpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <unordered_set>

class CBlockIndex;
class BlockValidationState;
class TxValidationState;

namespace Consensus
Expand Down
4 changes: 2 additions & 2 deletions src/evo/dmn_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
#define BITCOIN_EVO_DMN_TYPES_H

#include <amount.h>
#include <serialize.h>

#include <cassert>
#include <limits>
#include <string_view>

enum class MnType : uint16_t {
Expand All @@ -18,6 +17,7 @@ enum class MnType : uint16_t {
Invalid = std::numeric_limits<uint16_t>::max(),
};

template<typename T> struct is_serializable_enum;
template<> struct is_serializable_enum<MnType> : std::true_type {};

namespace dmn_types {
Expand Down
2 changes: 2 additions & 0 deletions src/evo/evodb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include <evo/evodb.h>

#include <uint256.h>

CEvoDBScopedCommitter::CEvoDBScopedCommitter(CEvoDB &_evoDB) :
evoDB(_evoDB)
{
Expand Down
2 changes: 1 addition & 1 deletion src/evo/evodb.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

#include <dbwrapper.h>
#include <sync.h>
#include <uint256.h>

class uint256;
// "b_b" was used in the initial version of deterministic MN storage
// "b_b2" was used after compact diffs were introduced
// "b_b3" was used after masternode type introduction in evoDB
Expand Down
1 change: 1 addition & 0 deletions src/evo/providertx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <consensus/validation.h>
#include <hash.h>
#include <script/standard.h>
#include <tinyformat.h>
#include <util/underlying.h>

bool CProRegTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const
Expand Down
1 change: 0 additions & 1 deletion src/evo/providertx.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <key_io.h>
#include <netaddress.h>
#include <pubkey.h>
#include <tinyformat.h>
#include <univalue.h>
#include <util/underlying.h>

Expand Down
1 change: 0 additions & 1 deletion src/evo/specialtx.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef BITCOIN_EVO_SPECIALTX_H
#define BITCOIN_EVO_SPECIALTX_H

#include <consensus/validation.h>
#include <primitives/transaction.h>
#include <serialize.h>
#include <streams.h>
Expand Down
42 changes: 42 additions & 0 deletions src/governance/exceptions.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) 2023 The Dash Core developers
// Distributed under the MIT/X11 software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <governance/exceptions.h>

#include <iostream>
#include <sstream>

std::ostream& operator<<(std::ostream& os, governance_exception_type_enum_t eType)
{
switch (eType) {
case GOVERNANCE_EXCEPTION_NONE:
os << "GOVERNANCE_EXCEPTION_NONE";
break;
case GOVERNANCE_EXCEPTION_WARNING:
os << "GOVERNANCE_EXCEPTION_WARNING";
break;
case GOVERNANCE_EXCEPTION_PERMANENT_ERROR:
os << "GOVERNANCE_EXCEPTION_PERMANENT_ERROR";
break;
case GOVERNANCE_EXCEPTION_TEMPORARY_ERROR:
os << "GOVERNANCE_EXCEPTION_TEMPORARY_ERROR";
break;
case GOVERNANCE_EXCEPTION_INTERNAL_ERROR:
os << "GOVERNANCE_EXCEPTION_INTERNAL_ERROR";
break;
}
return os;
}

CGovernanceException::CGovernanceException(const std::string& strMessageIn,
governance_exception_type_enum_t eTypeIn,
int nNodePenaltyIn) :
strMessage(),
eType(eTypeIn),
nNodePenalty(nNodePenaltyIn)
{
std::ostringstream ostr;
ostr << eType << ":" << strMessageIn;
strMessage = ostr.str();
}
Loading

0 comments on commit ba97f49

Please sign in to comment.