Compile eraseTree's search path length bookkeeping out of non-SHOW_MEMORY builds
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (pull_request) Successful in 5m8s
CI / pre-commit (pull_request) Successful in 6m12s
CI / release (arm64, , ubuntu-latest-arm64) (pull_request) Successful in 3m49s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (pull_request) Successful in 4m55s
CI / test (amd64, -DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 21, , 64-bit-versions, ubuntu-latest-amd64) (pull_request) Successful in 3m57s
CI / test (amd64, -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, 21, , gcc, ubuntu-latest-amd64) (pull_request) Successful in 3m54s
CI / test (amd64, -DUSE_SIMD_FALLBACK=ON, 21, , simd-fallback, ubuntu-latest-amd64) (pull_request) Successful in 3m34s
CI / release (amd64, -DMSAN_TOOLCHAIN_PATH=/opt/msan, ubuntu-latest-amd64) (pull_request) Successful in 5m56s
CI / coverage (pull_request) Successful in 3m58s

`eraseTree` only carries the search path lengths of queued nodes around so
that the SHOW_MEMORY key accounting doesn't have to walk `parent` pointers of
nodes that have already been released. Put the lengths on a parallel stack
that only exists when SHOW_MEMORY is enabled, so that other builds keep the
upstream worklist of bare `Node *` and don't compute anything extra.
This commit is contained in:
weaselbot committed 2026-10-27 12:56:55 -04:00
1 parent c0bd10effa
commit 80f084cc43
1 file changed
+51 -27
+51 -27
View File
@@ -2757,31 +2757,53 @@ checkMaxBetweenExclusiveImpl<true>(Node256 *n, int begin, int end,
return self;
}
#if SHOW_MEMORY
// Length of the key leading to `child`, given the length of the key leading to
// its parent. Consistent with `getSearchPathLength`.
int64_t childSearchPathLength(int64_t parentSearchPathLength, Node *child) {
return parentSearchPathLength + 1 + child->partialKeyLen;
}
#endif
void eraseTree(Node *root, WriteContext *writeContext) {
// A node is released before its children are visited, so the `parent`
// pointers of queued nodes can't be relied upon. Carry the search path length
// of each node along with it instead.
struct ToFree {
Node *n;
// Length of the key leading to `n`. See `getSearchPathLength`.
int64_t searchPathLength;
};
Arena arena;
auto toFree = vector<ToFree>(arena);
toFree.push_back({root, getSearchPathLength(root)});
auto toFree = vector<Node *>(arena);
toFree.push_back(root);
// Length of the key leading to each node in `toFree`, on a parallel stack.
// A node is released before its children are visited, so the `parent`
// pointers that `getSearchPathLength` walks are dangling by the time a queued
// node is popped. Carry the lengths along instead. Only the SHOW_MEMORY
// accounting needs them.
#if SHOW_MEMORY
auto searchPathLengths = vector<int64_t>(arena);
searchPathLengths.push_back(getSearchPathLength(root));
#endif
// Queue `child` for release, where `parentSearchPathLength` is the length of
// the key leading to its parent.
auto queueChild = [&](Node *child, int64_t parentSearchPathLength) {
toFree.push_back(child);
#if SHOW_MEMORY
searchPathLengths.push_back(
childSearchPathLength(parentSearchPathLength, child));
#else
(void)parentSearchPathLength;
#endif
};
while (toFree.size() > 0) {
const auto item = toFree.back();
auto *n = item.n;
auto *n = toFree.back();
toFree.pop_back();
writeContext->accum.entries_erased += n->entryPresent;
++writeContext->accum.nodes_released;
removeKey(n, item.searchPathLength);
// Search path length of the children of `n`. Must be read before `n` is
// released.
const int64_t childSearchPathLengthBase = item.searchPathLength + 1;
#if SHOW_MEMORY
const int64_t searchPathLength = searchPathLengths.back();
searchPathLengths.pop_back();
#else
const int64_t searchPathLength = 0;
#endif
removeKey(n, searchPathLength);
switch (n->getType()) {
case Type_Node0: {
@@ -2791,36 +2813,38 @@ void eraseTree(Node *root, WriteContext *writeContext) {
case Type_Node3: {
auto *n3 = static_cast<Node3 *>(n);
for (int i = 0; i < n3->numChildren; ++i) {
Node *child = n3->children[i];
toFree.push_back(
{child, childSearchPathLengthBase + child->partialKeyLen});
queueChild(n3->children[i], searchPathLength);
}
writeContext->release(n3);
} break;
case Type_Node16: {
auto *n16 = static_cast<Node16 *>(n);
for (int i = 0; i < n16->numChildren; ++i) {
Node *child = n16->children[i];
toFree.push_back(
{child, childSearchPathLengthBase + child->partialKeyLen});
queueChild(n16->children[i], searchPathLength);
}
writeContext->release(n16);
} break;
case Type_Node48: {
auto *n48 = static_cast<Node48 *>(n);
for (int i = 0; i < n48->numChildren; ++i) {
Node *child = n48->children[i];
toFree.push_back(
{child, childSearchPathLengthBase + child->partialKeyLen});
queueChild(n48->children[i], searchPathLength);
}
writeContext->release(n48);
} break;
case Type_Node256: {
auto *n256 = static_cast<Node256 *>(n);
auto *out = toFree.unsafePrepareAppend(n256->numChildren).data();
#if SHOW_MEMORY
auto *outSearchPathLengths =
searchPathLengths.unsafePrepareAppend(n256->numChildren).data();
#endif
n256->bitSet.forEachSet([&](int i) {
Node *child = n256->children[i];
*out++ = {child, childSearchPathLengthBase + child->partialKeyLen};
*out++ = child;
#if SHOW_MEMORY
*outSearchPathLengths++ =
childSearchPathLength(searchPathLength, child);
#endif
});
assert(out == toFree.end());
writeContext->release(n256);