Skip to content

Commit 3f5b5f3

Browse files
dongcarldongcarl
committed
validation: Move FindFilesToPrune{,Manual} to BlockManager
[META] No behaviour change is intended in this commit. [META] This commit should be followed up by removing the comments and assertions meant only to show that the change is correct. Also stop FindFilesToPrune{,Manual} from unnecessary reaching for ::ChainActive() by passing in the necessary information.
1 parent f8d4975 commit 3f5b5f3

File tree

2 files changed

+60
-16
lines changed

2 files changed

+60
-16
lines changed

‎src/validation.cpp‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,6 @@ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& loc
197197

198198
std::unique_ptr<CBlockTreeDB> pblocktree;
199199

200-
// See definition for documentation
201-
static void FindFilesToPruneManual(ChainstateManager& chainman, std::set<int>& setFilesToPrune, int nManualPruneHeight);
202-
static void FindFilesToPrune(ChainstateManager& chainman, std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight);
203200
bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks = nullptr);
204201
static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false);
205202
static FlatFileSeq BlockFileSeq();
@@ -2284,14 +2281,53 @@ bool CChainState::FlushStateToDisk(
22842281
CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool);
22852282
LOCK(cs_LastBlockFile);
22862283
if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
2284+
// Previously, we called the global function ::ChainActive() in
2285+
// FindFilesToPrune{,Manual} to get the tip height and to determine
2286+
// whether or not a tip even exists. Now, we are simply passing in
2287+
// m_chain.Height() (which returns -1 if the tip doesn't exist). To
2288+
// make sure we're not changing behaviour, let's check that
2289+
// ::ChainActive() is the same object as m_chain (not just
2290+
// identical).
2291+
//
2292+
// This comment and the following assert will be removed in a
2293+
// subsequent commit, as they're just meant to demonstrate
2294+
// correctness (you can run tests against it and see that nothing
2295+
// exit unexpectedly).
2296+
assert(std::addressof(::ChainActive()) == std::addressof(m_chain));
22872297
if (nManualPruneHeight > 0) {
22882298
LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCH);
22892299

2290-
FindFilesToPruneManual(g_chainman, setFilesToPrune, nManualPruneHeight);
2300+
m_blockman.FindFilesToPruneManual(setFilesToPrune, nManualPruneHeight, m_chain.Height());
22912301
} else {
22922302
LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCH);
22932303

2294-
FindFilesToPrune(g_chainman, setFilesToPrune, chainparams.PruneAfterHeight());
2304+
// Previously, we called the global function
2305+
// ::ChainstateActive() in FindFilesToPrune{,Manual} to get the
2306+
// IBD status. Now, we are simply passing in
2307+
// IsInitialBlockDownload(). To make sure we're not changing
2308+
// behaviour, let's check that ::ChainstateActive() is the same
2309+
// object as *this (not just identical).
2310+
//
2311+
// This comment and the following assert will be removed in a
2312+
// subsequent commit, as they're just meant to demonstrate
2313+
// correctness (you can run tests against it and see that
2314+
// nothing exit unexpectedly).
2315+
assert(std::addressof(::ChainstateActive()) == std::addressof(*this));
2316+
2317+
// Previously, we called PruneOneBlockFile on g_chainman's
2318+
// m_blockman in FindFilesToPrune{,Manual}. Now, we are instead
2319+
// calling PruneOneBlockFile on _our_ m_blockman. To make sure
2320+
// we're not changing behaviour, let's check that
2321+
// g_chainman.m_blockman is the same object as _our_ m_blockman
2322+
// (not just identical).
2323+
//
2324+
// This comment and the following assert will be removed in a
2325+
// subsequent commit, as they're just meant to demonstrate
2326+
// correctness (you can run tests against it and see that
2327+
// nothing exit unexpectedly).
2328+
assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_blockman));
2329+
2330+
m_blockman.FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight(), m_chain.Height(), IsInitialBlockDownload());
22952331
fCheckForPruning = false;
22962332
}
22972333
if (!setFilesToPrune.empty()) {
@@ -3958,21 +3994,21 @@ void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune)
39583994
}
39593995

39603996
/* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */
3961-
static void FindFilesToPruneManual(ChainstateManager& chainman, std::set<int>& setFilesToPrune, int nManualPruneHeight)
3997+
void BlockManager::FindFilesToPruneManual(std::set<int>& setFilesToPrune, int nManualPruneHeight, int chain_tip_height)
39623998
{
39633999
assert(fPruneMode && nManualPruneHeight > 0);
39644000

39654001
LOCK2(cs_main, cs_LastBlockFile);
3966-
if (::ChainActive().Tip() == nullptr)
4002+
if (chain_tip_height < 0)
39674003
return;
39684004

39694005
// last block to prune is the lesser of (user-specified height, MIN_BLOCKS_TO_KEEP from the tip)
3970-
unsigned int nLastBlockWeCanPrune = std::min((unsigned)nManualPruneHeight, ::ChainActive().Tip()->nHeight - MIN_BLOCKS_TO_KEEP);
4006+
unsigned int nLastBlockWeCanPrune = std::min((unsigned)nManualPruneHeight, chain_tip_height - MIN_BLOCKS_TO_KEEP);
39714007
int count=0;
39724008
for (int fileNumber = 0; fileNumber < nLastBlockFile; fileNumber++) {
39734009
if (vinfoBlockFile[fileNumber].nSize == 0 || vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune)
39744010
continue;
3975-
chainman.m_blockman.PruneOneBlockFile(fileNumber);
4011+
PruneOneBlockFile(fileNumber);
39764012
setFilesToPrune.insert(fileNumber);
39774013
count++;
39784014
}
@@ -4005,17 +4041,17 @@ void PruneBlockFilesManual(int nManualPruneHeight)
40054041
*
40064042
* @param[out] setFilesToPrune The set of file indices that can be unlinked will be returned
40074043
*/
4008-
static void FindFilesToPrune(ChainstateManager& chainman, std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight)
4044+
void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd)
40094045
{
40104046
LOCK2(cs_main, cs_LastBlockFile);
4011-
if (::ChainActive().Tip() == nullptr || nPruneTarget == 0) {
4047+
if (chain_tip_height < 0 || nPruneTarget == 0) {
40124048
return;
40134049
}
4014-
if ((uint64_t)::ChainActive().Tip()->nHeight <= nPruneAfterHeight) {
4050+
if ((uint64_t)chain_tip_height <= nPruneAfterHeight) {
40154051
return;
40164052
}
40174053

4018-
unsigned int nLastBlockWeCanPrune = ::ChainActive().Tip()->nHeight - MIN_BLOCKS_TO_KEEP;
4054+
unsigned int nLastBlockWeCanPrune = chain_tip_height - MIN_BLOCKS_TO_KEEP;
40194055
uint64_t nCurrentUsage = CalculateCurrentUsage();
40204056
// We don't check to prune until after we've allocated new space for files
40214057
// So we should leave a buffer under our target to account for another allocation
@@ -4029,7 +4065,7 @@ static void FindFilesToPrune(ChainstateManager& chainman, std::set<int>& setFile
40294065
// To avoid excessive prune events negating the benefit of high dbcache
40304066
// values, we should not prune too rapidly.
40314067
// So when pruning in IBD, increase the buffer a bit to avoid a re-prune too soon.
4032-
if (::ChainstateActive().IsInitialBlockDownload()) {
4068+
if (is_ibd) {
40334069
// Since this is only relevant during IBD, we use a fixed 10%
40344070
nBuffer += nPruneTarget / 10;
40354071
}
@@ -4047,7 +4083,7 @@ static void FindFilesToPrune(ChainstateManager& chainman, std::set<int>& setFile
40474083
if (vinfoBlockFile[fileNumber].nHeightLast > nLastBlockWeCanPrune)
40484084
continue;
40494085

4050-
chainman.m_blockman.PruneOneBlockFile(fileNumber);
4086+
PruneOneBlockFile(fileNumber);
40514087
// Queue up the files for removal
40524088
setFilesToPrune.insert(fileNumber);
40534089
nCurrentUsage -= nBytesToPrune;

‎src/validation.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,15 @@ struct CBlockIndexWorkComparator
356356
* This data is used mostly in `CChainState` - information about, e.g.,
357357
* candidate tips is not maintained here.
358358
*/
359-
class BlockManager {
359+
class BlockManager
360+
{
361+
friend CChainState;
362+
363+
private:
364+
// See definition for documentation
365+
void FindFilesToPruneManual(std::set<int>& setFilesToPrune, int nManualPruneHeight, int chain_tip_height);
366+
void FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, bool is_ibd);
367+
360368
public:
361369
BlockMap m_block_index GUARDED_BY(cs_main);
362370

0 commit comments

Comments
 (0)