Skip to content

Commit 8d2ead2

Browse files
committed
Merge bitcoin/bitcoin#32185: coins: replace manual CDBBatch size estimation with LevelDB's native ApproximateSize
e419b0e refactor: Remove manual CDBBatch size estimation (Lőrinc) 8b5e19d refactor: Delegate to LevelDB for CDBBatch size estimation (Lőrinc) 751077c Coins: Add `kHeader` to `CDBBatch::size_estimate` (Lőrinc) Pull request description: ### Summary The manual batch size estimation of `CDBBatch` serialized size was [added](bitcoin/bitcoin@e66dbde) when LevelDB [didn't expose this functionality yet](google/leveldb@69e2bd2). The PR refactors the logic to use the native `leveldb::WriteBatch::ApproximateSize()` function, structured in 3 focused commits to incrementally replace the old behavior safely. ### Context The previous manual size calculation initialized the estimate to 0, instead of LevelDB's header size (containing an 8-byte sequence number followed by a 4-byte count). This PR corrects that and transitions to the now-available native LevelDB function for improved accuracy and maintainability. ### Approach The fix and refactor follow a strangle pattern over three commits: * correct the initialization bug in the existing manual calculation, isolating the fix and ensuring the subsequent assertions use the corrected logic; * introduce the native `ApproximateSize()` method alongside the corrected manual one, adding assertions to verify their equivalence at runtime; * remove the verified manual calculation logic and assertions, leaving only the native method. ACKs for top commit: sipa: utACK e419b0e TheCharlatan: ACK e419b0e laanwj: Code review ACK e419b0e Tree-SHA512: a12b973dd480d4ffec4ec89a119bf0b6f73bde4e634329d6e4cc3454b867f2faf3742b78ec4a3b6d98ac4fb28fb2174f44ede42d6c701ed871987a7274560691
2 parents ad0eee5 + e419b0e commit 8d2ead2

File tree

3 files changed

+18
-24
lines changed

3 files changed

+18
-24
lines changed

src/dbwrapper.cpp

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,16 @@ struct CDBBatch::WriteBatchImpl {
158158

159159
CDBBatch::CDBBatch(const CDBWrapper& _parent)
160160
: parent{_parent},
161-
m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()} {};
161+
m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()}
162+
{
163+
Clear();
164+
};
162165

163166
CDBBatch::~CDBBatch() = default;
164167

165168
void CDBBatch::Clear()
166169
{
167170
m_impl_batch->batch.Clear();
168-
size_estimate = 0;
169171
}
170172

171173
void CDBBatch::WriteImpl(std::span<const std::byte> key, DataStream& ssValue)
@@ -174,26 +176,17 @@ void CDBBatch::WriteImpl(std::span<const std::byte> key, DataStream& ssValue)
174176
ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent));
175177
leveldb::Slice slValue(CharCast(ssValue.data()), ssValue.size());
176178
m_impl_batch->batch.Put(slKey, slValue);
177-
// LevelDB serializes writes as:
178-
// - byte: header
179-
// - varint: key length (1 byte up to 127B, 2 bytes up to 16383B, ...)
180-
// - byte[]: key
181-
// - varint: value length
182-
// - byte[]: value
183-
// The formula below assumes the key and value are both less than 16k.
184-
size_estimate += 3 + (slKey.size() > 127) + slKey.size() + (slValue.size() > 127) + slValue.size();
185179
}
186180

187181
void CDBBatch::EraseImpl(std::span<const std::byte> key)
188182
{
189183
leveldb::Slice slKey(CharCast(key.data()), key.size());
190184
m_impl_batch->batch.Delete(slKey);
191-
// LevelDB serializes erases as:
192-
// - byte: header
193-
// - varint: key length
194-
// - byte[]: key
195-
// The formula below assumes the key is less than 16kB.
196-
size_estimate += 2 + (slKey.size() > 127) + slKey.size();
185+
}
186+
187+
size_t CDBBatch::ApproximateSize() const
188+
{
189+
return m_impl_batch->batch.ApproximateSize();
197190
}
198191

199192
struct LevelDBContext {

src/dbwrapper.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ class CDBBatch
8383
DataStream ssKey{};
8484
DataStream ssValue{};
8585

86-
size_t size_estimate{0};
87-
8886
void WriteImpl(std::span<const std::byte> key, DataStream& ssValue);
8987
void EraseImpl(std::span<const std::byte> key);
9088

@@ -117,7 +115,7 @@ class CDBBatch
117115
ssKey.clear();
118116
}
119117

120-
size_t SizeEstimate() const { return size_estimate; }
118+
size_t ApproximateSize() const;
121119
};
122120

123121
class CDBIterator

src/txdb.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,16 +119,19 @@ bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashB
119119
for (auto it{cursor.Begin()}; it != cursor.End();) {
120120
if (it->second.IsDirty()) {
121121
CoinEntry entry(&it->first);
122-
if (it->second.coin.IsSpent())
122+
if (it->second.coin.IsSpent()) {
123123
batch.Erase(entry);
124-
else
124+
} else {
125125
batch.Write(entry, it->second.coin);
126+
}
127+
126128
changed++;
127129
}
128130
count++;
129131
it = cursor.NextAndMaybeErase(*it);
130-
if (batch.SizeEstimate() > m_options.batch_write_bytes) {
131-
LogDebug(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0));
132+
if (batch.ApproximateSize() > m_options.batch_write_bytes) {
133+
LogDebug(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.ApproximateSize() * (1.0 / 1048576.0));
134+
132135
m_db->WriteBatch(batch);
133136
batch.Clear();
134137
if (m_options.simulate_crash_ratio) {
@@ -145,7 +148,7 @@ bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashB
145148
batch.Erase(DB_HEAD_BLOCKS);
146149
batch.Write(DB_BEST_BLOCK, hashBlock);
147150

148-
LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0));
151+
LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.ApproximateSize() * (1.0 / 1048576.0));
149152
bool ret = m_db->WriteBatch(batch);
150153
LogDebug(BCLog::COINDB, "Committed %u changed transaction outputs (out of %u) to coin database...\n", (unsigned int)changed, (unsigned int)count);
151154
return ret;

0 commit comments

Comments
 (0)