Compare commits

...
Author SHA1 Message Date
weaselbot b671f93e5a Document that SHOW_MEMORY assumes a single live ConflictSet
The testing-only cross-check in `internal_addWrites` and
`internal_setOldestVersion` compares `impl->totalBytes`, which only accounts for
the allocations made during one instance's own API calls, against `mallocBytes`,
which is process-wide. The two are only equal while a single ConflictSet uses
`safe_malloc`, so a SHOW_MEMORY build aborts on perfectly valid programs that
keep two instances alive (the repo's own script tests do exactly that).

Keeping several ConflictSets alive is supported and does not need the
instrumentation fixed for it, so just say so where the invariant lives: on the
process-wide counters in `Internal.h`, and at each of the four cross-checks in
ConflictSet.cpp and SkipList.cpp. ShowMemoryTest.cpp's header comment and the
CMakeLists comment for the test target now point at that explanation instead of
stating the restriction without a reason.

Closes #97
2026-11-10 15:37:54 -05:00
5 changed files with 24 additions and 5 deletions

No files matched your search

+1 -1
View File
@@ -394,7 +394,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
add_test(NAME conflict_set_cxx_api_test COMMAND conflict_set_cxx_api_test)
# SHOW_MEMORY instrumentation, which the rest of the test suite doesn't
# compile with
# compile with. It assumes a single live ConflictSet (see Internal.h).
add_executable(conflict_set_show_memory_test ConflictSet.cpp
ShowMemoryTest.cpp)
target_compile_options(conflict_set_show_memory_test PRIVATE ${TEST_FLAGS})
+6 -2
View File
@@ -5546,7 +5546,9 @@ void internal_addWrites(ConflictSet::Impl *impl,
impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY
// Testing-only cross-check of the accounting.
// Testing-only cross-check of the accounting. `mallocBytes` is process-wide,
// while `impl->totalBytes` only covers this instance, so this holds only
// while this is the only live ConflictSet (see Internal.h).
if (impl->totalBytes != mallocBytes) {
abort(); // GCOVR_EXCL_LINE
}
@@ -5559,7 +5561,9 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY
// Testing-only cross-check of the accounting.
// Testing-only cross-check of the accounting. `mallocBytes` is process-wide,
// while `impl->totalBytes` only covers this instance, so this holds only
// while this is the only live ConflictSet (see Internal.h).
if (impl->totalBytes != mallocBytes) {
abort(); // GCOVR_EXCL_LINE
}
+8
View File
@@ -96,6 +96,14 @@ static_assert(std::is_trivial_v<TrivialSpan>);
// GCOVR_EXCL_START
#if SHOW_MEMORY
// Process-wide by design: `safe_malloc`/`safe_free` update these for every
// allocation in the process, no matter which ConflictSet made it. An instance's
// `totalBytes`, in contrast, only accounts for the allocations made during its
// own API calls, so the SHOW_MEMORY cross-check of the two in
// `internal_addWrites`/`internal_setOldestVersion` only holds while a single
// ConflictSet is alive. Running several at once is supported by the library;
// SHOW_MEMORY just doesn't instrument it. `peakMallocBytes` is process-wide
// too, but as a peak it remains meaningful with several instances.
inline int64_t mallocBytes = 0;
inline int64_t peakMallocBytes = 0;
// Defined in ConflictSet.cpp
+3 -2
View File
@@ -1,8 +1,9 @@
// Exercises the SHOW_MEMORY instrumentation, which the other tests don't
// compile with (this target passes -DSHOW_MEMORY=1). The instrumentation has
// its own consistency checks, which abort on failure, and asserts are enabled
// by TEST_FLAGS. Only one ConflictSet may exist at a time, since the
// accounting is process-wide.
// by TEST_FLAGS. Only one ConflictSet may exist at a time: the accounting is
// process-wide, while the cross-check compares it against a single instance's
// totals (see the comment on `mallocBytes` in Internal.h).
#include "ConflictSet.h"
#include "Internal.h"
+6
View File
@@ -915,6 +915,9 @@ void internal_addWrites(ConflictSet::Impl *impl,
impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY
// Testing-only cross-check of the accounting. `mallocBytes` is process-wide,
// while `impl->totalBytes` only covers this instance, so this holds only
// while this is the only live ConflictSet (see Internal.h).
if (impl->totalBytes != mallocBytes) {
abort();
}
@@ -927,6 +930,9 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY
// Testing-only cross-check of the accounting. `mallocBytes` is process-wide,
// while `impl->totalBytes` only covers this instance, so this holds only
// while this is the only live ConflictSet (see Internal.h).
if (impl->totalBytes != mallocBytes) {
abort();
}