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
This commit is contained in:
weaselbot committed 2026-11-10 15:37:54 -05:00
1 parent e23018c952
commit b671f93e5a
5 files changed
+24 -5

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) 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 # 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 add_executable(conflict_set_show_memory_test ConflictSet.cpp
ShowMemoryTest.cpp) ShowMemoryTest.cpp)
target_compile_options(conflict_set_show_memory_test PRIVATE ${TEST_FLAGS}) 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->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes); impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY #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) { if (impl->totalBytes != mallocBytes) {
abort(); // GCOVR_EXCL_LINE abort(); // GCOVR_EXCL_LINE
} }
@@ -5559,7 +5561,9 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
impl->totalBytes += mallocBytesDelta; impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes); impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY #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) { if (impl->totalBytes != mallocBytes) {
abort(); // GCOVR_EXCL_LINE abort(); // GCOVR_EXCL_LINE
} }
+8
View File
@@ -96,6 +96,14 @@ static_assert(std::is_trivial_v<TrivialSpan>);
// GCOVR_EXCL_START // GCOVR_EXCL_START
#if SHOW_MEMORY #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 mallocBytes = 0;
inline int64_t peakMallocBytes = 0; inline int64_t peakMallocBytes = 0;
// Defined in ConflictSet.cpp // Defined in ConflictSet.cpp
+3 -2
View File
@@ -1,8 +1,9 @@
// Exercises the SHOW_MEMORY instrumentation, which the other tests don't // Exercises the SHOW_MEMORY instrumentation, which the other tests don't
// compile with (this target passes -DSHOW_MEMORY=1). The instrumentation has // compile with (this target passes -DSHOW_MEMORY=1). The instrumentation has
// its own consistency checks, which abort on failure, and asserts are enabled // 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 // by TEST_FLAGS. Only one ConflictSet may exist at a time: the accounting is
// accounting is process-wide. // 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 "ConflictSet.h"
#include "Internal.h" #include "Internal.h"
+6
View File
@@ -915,6 +915,9 @@ void internal_addWrites(ConflictSet::Impl *impl,
impl->totalBytes += mallocBytesDelta; impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes); impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY #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) { if (impl->totalBytes != mallocBytes) {
abort(); abort();
} }
@@ -927,6 +930,9 @@ void internal_setOldestVersion(ConflictSet::Impl *impl, int64_t oldestVersion) {
impl->totalBytes += mallocBytesDelta; impl->totalBytes += mallocBytesDelta;
impl->memory_bytes.set(impl->totalBytes); impl->memory_bytes.set(impl->totalBytes);
#if SHOW_MEMORY #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) { if (impl->totalBytes != mallocBytes) {
abort(); abort();
} }