As suggested in the issue discussion, this doesn't try to make the SHOW_MEMORY cross-check work with several instances, it documents that it assumes one at a time.
internal_addWrites and internal_setOldestVersion compare two different scopes: impl->totalBytes, which accumulates only what this instance allocated during its own API calls, and mallocBytes, a process-wide counter that safe_malloc/safe_free bump for everybody. They can only be equal while a single ConflictSet is alive, so a SHOW_MEMORY build aborts on valid programs with two instances (and on the repo's own test_update_zero_* script tests).
Internal.h: the comment on mallocBytes now explains that it is process-wide, that impl->totalBytes is not, and that the cross-check therefore assumes a single live instance — running several is supported, SHOW_MEMORY just doesn't instrument it. peakMallocBytes is a peak, so it stays meaningful either way.
ConflictSet.cpp and SkipList.cpp: each of the four cross-checks says what it assumes and points at that explanation.
ShowMemoryTest.cpp / CMakeLists.txt: the single-instance restriction is now given a reason instead of just being asserted.
No behaviour change; the instrumentation keeps its cross-check for the single-instance case it was written for.
Tested: full ctest -j8 on a Debug build (8601/8601 pass, including conflict_set_show_memory_test and the script_test_* targets), and both ConflictSet.cpp and SkipList.cpp still compile cleanly with and without -DSHOW_MEMORY=1.
Closes #97
As suggested in the issue discussion, this doesn't try to make the SHOW_MEMORY cross-check work with several instances, it documents that it assumes one at a time.
`internal_addWrites` and `internal_setOldestVersion` compare two different scopes: `impl->totalBytes`, which accumulates only what this instance allocated during its own API calls, and `mallocBytes`, a process-wide counter that `safe_malloc`/`safe_free` bump for everybody. They can only be equal while a single `ConflictSet` is alive, so a SHOW_MEMORY build aborts on valid programs with two instances (and on the repo's own `test_update_zero_*` script tests).
- `Internal.h`: the comment on `mallocBytes` now explains that it is process-wide, that `impl->totalBytes` is not, and that the cross-check therefore assumes a single live instance — running several is supported, SHOW_MEMORY just doesn't instrument it. `peakMallocBytes` is a peak, so it stays meaningful either way.
- `ConflictSet.cpp` and `SkipList.cpp`: each of the four cross-checks says what it assumes and points at that explanation.
- `ShowMemoryTest.cpp` / `CMakeLists.txt`: the single-instance restriction is now given a reason instead of just being asserted.
No behaviour change; the instrumentation keeps its cross-check for the single-instance case it was written for.
Tested: full `ctest -j8` on a Debug build (8601/8601 pass, including `conflict_set_show_memory_test` and the `script_test_*` targets), and both `ConflictSet.cpp` and `SkipList.cpp` still compile cleanly with and without `-DSHOW_MEMORY=1`.
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 4m26s
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 4m3s
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
All checks were successful
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 4m26s
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 4m3s
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #97
As suggested in the issue discussion, this doesn't try to make the SHOW_MEMORY cross-check work with several instances, it documents that it assumes one at a time.
internal_addWritesandinternal_setOldestVersioncompare two different scopes:impl->totalBytes, which accumulates only what this instance allocated during its own API calls, andmallocBytes, a process-wide counter thatsafe_malloc/safe_freebump for everybody. They can only be equal while a singleConflictSetis alive, so a SHOW_MEMORY build aborts on valid programs with two instances (and on the repo's owntest_update_zero_*script tests).Internal.h: the comment onmallocBytesnow explains that it is process-wide, thatimpl->totalBytesis not, and that the cross-check therefore assumes a single live instance — running several is supported, SHOW_MEMORY just doesn't instrument it.peakMallocBytesis a peak, so it stays meaningful either way.ConflictSet.cppandSkipList.cpp: each of the four cross-checks says what it assumes and points at that explanation.ShowMemoryTest.cpp/CMakeLists.txt: the single-instance restriction is now given a reason instead of just being asserted.No behaviour change; the instrumentation keeps its cross-check for the single-instance case it was written for.
Tested: full
ctest -j8on a Debug build (8601/8601 pass, includingconflict_set_show_memory_testand thescript_test_*targets), and bothConflictSet.cppandSkipList.cppstill compile cleanly with and without-DSHOW_MEMORY=1.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.