From 96c4acd5dd83b22be9f5589dd9cee6bea01fbca2 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Sun, 11 Aug 2019 07:54:09 +0100 Subject: [PATCH 01/11] Make all functional tests use statistics. --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index eeab751..e94965a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -174,6 +174,9 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) if (${TEST_CATEGORY} MATCHES "perf") set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) endif() + if (${TEST_CATEGORY} MATCHES "func") + target_compile_definitions(${TESTNAME} PRIVATE -DUSE_SNMALLOC_STATS) + endif () endforeach() endforeach() endforeach() From a32882cd5599071f9e9403071abcb33843b1187f Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Sun, 11 Aug 2019 07:54:51 +0100 Subject: [PATCH 02/11] Make malloc functional test check for leaks. --- src/test/func/malloc/malloc.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/func/malloc/malloc.cc b/src/test/func/malloc/malloc.cc index e692f3c..8974e3a 100644 --- a/src/test/func/malloc/malloc.cc +++ b/src/test/func/malloc/malloc.cc @@ -115,5 +115,6 @@ int main(int argc, char** argv) test_posix_memalign(0, align + 1, EINVAL, true); } + current_alloc_pool()->debug_check_empty(); return 0; } From 6151b7a9b23d8d9b7c7e8eaabcfca64f82512801 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Sun, 11 Aug 2019 07:44:36 +0100 Subject: [PATCH 03/11] Make test_realloc not leak in failure case. --- src/test/func/malloc/malloc.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/func/malloc/malloc.cc b/src/test/func/malloc/malloc.cc index 8974e3a..b3dd268 100644 --- a/src/test/func/malloc/malloc.cc +++ b/src/test/func/malloc/malloc.cc @@ -48,8 +48,11 @@ void test_realloc(void* p, size_t size, int err, bool null) { fprintf(stderr, "realloc(%p(%d), %d)\n", p, int(size), (int)size); errno = 0; - p = our_realloc(p, size); - check_result(size, 1, p, err, null); + auto new_p = our_realloc(p, size); + // Realloc failure case, deallocate original block + if (new_p == nullptr && size != 0) + our_free(p); + check_result(size, 1, new_p, err, null); } void test_posix_memalign(size_t size, size_t align, int err, bool null) From cbb1063e8257923ba4138d53ae7385056d48279e Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 12 Aug 2019 18:16:11 +0100 Subject: [PATCH 04/11] Fix up checking for empty to account for stub messages. --- src/mem/globalalloc.h | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/src/mem/globalalloc.h b/src/mem/globalalloc.h index bda5c35..cf90567 100644 --- a/src/mem/globalalloc.h +++ b/src/mem/globalalloc.h @@ -107,41 +107,43 @@ namespace snmalloc #endif } - void debug_check_empty() + /** + If you pass a pointer to a bool, then it returns whether all the allocators are empty. + If you don't pass a pointer to a bool, then will raise an error all the allocators are + not empty. + */ + void debug_check_empty(bool* result = nullptr) { #ifndef USE_MALLOC // This is a debugging function. It checks that all memory from all // allocators has been freed. - size_t alloc_count = 0; - auto* alloc = Parent::iterate(); - // Count the linked allocators. - while (alloc != nullptr) - { - alloc = Parent::iterate(alloc); - alloc_count++; - } - bool done = false; + size_t non_empty_count = 0; while (!done) { done = true; alloc = Parent::iterate(); + non_empty_count = 0; while (alloc != nullptr) { // Destroy the message queue so that it has no stub message. Remote* p = alloc->message_queue().destroy(); - while (p != nullptr) + while (p != nullptr) { Remote* next = p->non_atomic_next; alloc->handle_dealloc_remote(p); p = next; } + // Check that the allocator has freed all memory. + if (!alloc->stats().is_empty()) + non_empty_count++; + // Place the static stub message on the queue. alloc->init_message_queue(); @@ -158,20 +160,16 @@ namespace snmalloc } } - alloc = Parent::iterate(); - size_t empty_count = 0; - - while (alloc != nullptr) + if (result != nullptr) { - // Check that the allocator has freed all memory. - if (alloc->stats().is_empty()) - empty_count++; - - alloc = Parent::iterate(alloc); + *result = non_empty_count == 0; + return; } - if (alloc_count != empty_count) - error("Incorrect number of allocators"); + if (non_empty_count != 0) + { + error("debug_check_empty: found non-empty allocators"); + } #endif } }; From e6b4efd9805bf76259ef984f6c35fed19970fcf1 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 12 Aug 2019 18:15:31 +0100 Subject: [PATCH 05/11] Don't apply changes to GlobalPlaceholder. --- src/mem/alloc.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mem/alloc.h b/src/mem/alloc.h index 155f01a..97b0171 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -1068,13 +1068,13 @@ namespace snmalloc SNMALLOC_ASSUME(size <= SLAB_SIZE); sizeclass_t sizeclass = size_to_sizeclass(size); - stats().sizeclass_alloc(sizeclass); assert(sizeclass < NUM_SMALL_CLASSES); auto& fl = small_fast_free_lists[sizeclass]; void* head = fl.value; if (likely(head != nullptr)) { + stats().sizeclass_alloc(sizeclass); // Read the next slot from the memory that's about to be allocated. fl.value = Metaslab::follow_next(head); @@ -1097,6 +1097,9 @@ namespace snmalloc return reinterpret_cast(replacement) ->template small_alloc_slow(sizeclass); } + + stats().sizeclass_alloc(sizeclass); + handle_message_queue(); size_t rsize = sizeclass_to_size(sizeclass); auto& sl = small_classes[sizeclass]; From f545c1c79091aac2fda38df3bb4ebc1aec08ce4a Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 13 Aug 2019 10:08:11 +0100 Subject: [PATCH 06/11] Test for debug_check_emtpy. --- src/test/func/statistics/stats.cc | 41 +++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 src/test/func/statistics/stats.cc diff --git a/src/test/func/statistics/stats.cc b/src/test/func/statistics/stats.cc new file mode 100644 index 0000000..76ac2f1 --- /dev/null +++ b/src/test/func/statistics/stats.cc @@ -0,0 +1,41 @@ +#include + + + +int main() +{ + snmalloc::Alloc* a = snmalloc::ThreadAlloc::get(); + bool result; + + auto r = a->alloc(16); + + snmalloc::current_alloc_pool()->debug_check_empty(&result); + if (result != false) + { + abort(); + } + + a->dealloc(r); + + snmalloc::current_alloc_pool()->debug_check_empty(&result); + if (result != true) + { + abort(); + } + + r = a->alloc(16); + + snmalloc::current_alloc_pool()->debug_check_empty(&result); + if (result != false) + { + abort(); + } + + a->dealloc(r); + + snmalloc::current_alloc_pool()->debug_check_empty(&result); + if (result != true) + { + abort(); + } +} \ No newline at end of file From 2ab4bb43663e18bcf685b670a5e556dabf5f5c22 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 13 Aug 2019 10:56:54 +0100 Subject: [PATCH 07/11] Remove new from examples as they don't use it. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e94965a..bd3a275 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -158,7 +158,7 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC) set(TESTNAME "${TEST_CATEGORY}-${TEST}-${SUPER_SLAB_SIZE}") - add_executable(${TESTNAME} ${SRC} src/override/new.cc) + add_executable(${TESTNAME} ${SRC}) if (${SUPER_SLAB_SIZE} EQUAL 1) target_compile_definitions(${TESTNAME} PRIVATE IS_ADDRESS_SPACE_CONSTRAINED) endif() From bef2fb94d86fd078dff35b5734bb4a1a692ca792 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 13 Aug 2019 13:27:33 +0100 Subject: [PATCH 08/11] Clangformat --- src/mem/globalalloc.h | 8 ++++---- src/test/func/statistics/stats.cc | 8 +++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/mem/globalalloc.h b/src/mem/globalalloc.h index cf90567..591a70b 100644 --- a/src/mem/globalalloc.h +++ b/src/mem/globalalloc.h @@ -108,9 +108,9 @@ namespace snmalloc } /** - If you pass a pointer to a bool, then it returns whether all the allocators are empty. - If you don't pass a pointer to a bool, then will raise an error all the allocators are - not empty. + If you pass a pointer to a bool, then it returns whether all the + allocators are empty. If you don't pass a pointer to a bool, then will + raise an error all the allocators are not empty. */ void debug_check_empty(bool* result = nullptr) { @@ -133,7 +133,7 @@ namespace snmalloc // Destroy the message queue so that it has no stub message. Remote* p = alloc->message_queue().destroy(); - while (p != nullptr) + while (p != nullptr) { Remote* next = p->non_atomic_next; alloc->handle_dealloc_remote(p); diff --git a/src/test/func/statistics/stats.cc b/src/test/func/statistics/stats.cc index 76ac2f1..6ecbe09 100644 --- a/src/test/func/statistics/stats.cc +++ b/src/test/func/statistics/stats.cc @@ -1,12 +1,10 @@ #include - - int main() { snmalloc::Alloc* a = snmalloc::ThreadAlloc::get(); bool result; - + auto r = a->alloc(16); snmalloc::current_alloc_pool()->debug_check_empty(&result); @@ -19,7 +17,7 @@ int main() snmalloc::current_alloc_pool()->debug_check_empty(&result); if (result != true) - { + { abort(); } @@ -35,7 +33,7 @@ int main() snmalloc::current_alloc_pool()->debug_check_empty(&result); if (result != true) - { + { abort(); } } \ No newline at end of file From dd3feb948c49e14f779580c09171f10ea42c11ba Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 13 Aug 2019 13:43:20 +0100 Subject: [PATCH 09/11] Deal with wrap around in statistics. --- src/mem/allocstats.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mem/allocstats.h b/src/mem/allocstats.h index 5519985..7b68ae7 100644 --- a/src/mem/allocstats.h +++ b/src/mem/allocstats.h @@ -141,7 +141,9 @@ namespace snmalloc UNUSED(size); #ifdef USE_SNMALLOC_STATS - bucketed_requests[bits::to_exp_mant(size)]++; + auto index = (size == 0) ? 0 : bits::to_exp_mant(size); + assert(index < TOTAL_BUCKETS); + bucketed_requests[index]++; #endif } From 16b084f5010d49bc83e0d1498009b323f96f7702 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 13 Aug 2019 15:37:54 +0100 Subject: [PATCH 10/11] Changed abort behaviour for Windows CI. --- CMakeLists.txt | 5 +++++ azure-pipelines.yml | 12 +++++------ src/test/func/malloc/malloc.cc | 3 +++ src/test/func/release-rounding/rounding.cc | 3 +++ src/test/func/sizeclass/sizeclass.cc | 3 +++ src/test/func/two_alloc_types/main.cc | 3 +++ src/test/perf/contention/contention.cc | 3 +++ .../perf/external_pointer/externalpointer.cc | 3 +++ src/test/perf/singlethread/singlethread.cc | 3 +++ src/test/setup.h | 20 +++++++++++++++++++ 10 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 src/test/setup.h diff --git a/CMakeLists.txt b/CMakeLists.txt index bd3a275..fe74d39 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.8) project(snmalloc C CXX) option(USE_SNMALLOC_STATS "Track allocation stats" OFF) +option(SNMALLOC_CI_BUILD "Disable features not sensible for CI" OFF) option(USE_MEASURE "Measure performance with histograms" OFF) option(EXPOSE_EXTERNAL_PAGEMAP "Expose the global pagemap" OFF) option(EXPOSE_EXTERNAL_RESERVE "Expose an interface to reserve memory using the default memory provider" OFF) @@ -95,6 +96,10 @@ if(USE_MEASURE) target_compile_definitions(snmalloc_lib INTERFACE -DUSE_MEASURE) endif() +if(SNMALLOC_CI_BUILD) + target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_CI_BUILD) +endif() + if(CACHE_FRIENDLY_OFFSET) target_compile_definitions(snmalloc_lib INTERFACE -DCACHE_FRIENDLY_OFFSET=${CACHE_FRIENDLY_OFFSET}) endif() diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f7ce06c..2e59630 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -72,9 +72,9 @@ jobs: displayName: 'Install Build Dependencies' - task: CMake@1 - displayName: 'CMake .. $(CMakeArgs) -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"' + displayName: 'CMake .. $(CMakeArgs) -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS" -DSNMALLOC_CI_BUILD=On' inputs: - cmakeArgs: '.. $(CMakeArgs) -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"' + cmakeArgs: '.. $(CMakeArgs) -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS" -DSNMALLOC_CI_BUILD=On' - script: | ninja @@ -131,9 +131,9 @@ jobs: steps: - task: CMake@1 - displayName: 'CMake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType)' + displayName: 'CMake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On' inputs: - cmakeArgs: '.. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType)' + cmakeArgs: '.. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On' - task: MSBuild@1 displayName: 'Build solution build/snmalloc.sln' @@ -158,9 +158,9 @@ jobs: steps: - task: CMake@1 - displayName: 'CMake .. -DCMAKE_BUILD_TYPE=$(BuildType)' + displayName: 'CMake .. -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On' inputs: - cmakeArgs: '.. -DCMAKE_BUILD_TYPE=$(BuildType)' + cmakeArgs: '.. -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On' - script: | make -j 4 diff --git a/src/test/func/malloc/malloc.cc b/src/test/func/malloc/malloc.cc index b3dd268..8df3455 100644 --- a/src/test/func/malloc/malloc.cc +++ b/src/test/func/malloc/malloc.cc @@ -1,4 +1,5 @@ #include +#include #define SNMALLOC_NAME_MANGLE(a) our_##a #include "../../../override/malloc.cc" @@ -76,6 +77,8 @@ int main(int argc, char** argv) UNUSED(argc); UNUSED(argv); + setup(); + constexpr int SUCCESS = 0; test_calloc(0, 0, SUCCESS, false); diff --git a/src/test/func/release-rounding/rounding.cc b/src/test/func/release-rounding/rounding.cc index 8c3a9e8..0836319 100644 --- a/src/test/func/release-rounding/rounding.cc +++ b/src/test/func/release-rounding/rounding.cc @@ -1,4 +1,5 @@ #include +#include using namespace snmalloc; @@ -9,6 +10,8 @@ using namespace snmalloc; int main(int argc, char** argv) { + setup(); + UNUSED(argc); UNUSED(argv); diff --git a/src/test/func/sizeclass/sizeclass.cc b/src/test/func/sizeclass/sizeclass.cc index 6ca6c8d..718b73c 100644 --- a/src/test/func/sizeclass/sizeclass.cc +++ b/src/test/func/sizeclass/sizeclass.cc @@ -1,5 +1,6 @@ #include #include +#include NOINLINE snmalloc::sizeclass_t size_to_sizeclass(size_t size) @@ -9,6 +10,8 @@ snmalloc::sizeclass_t size_to_sizeclass(size_t size) int main(int, char**) { + setup(); + bool failed = false; size_t size_low = 0; diff --git a/src/test/func/two_alloc_types/main.cc b/src/test/func/two_alloc_types/main.cc index c15438b..9802b7c 100644 --- a/src/test/func/two_alloc_types/main.cc +++ b/src/test/func/two_alloc_types/main.cc @@ -3,6 +3,7 @@ #include #include #include +#include void* oe_base; void* oe_end; @@ -40,6 +41,8 @@ host_snmalloc_pagemap_global_get(snmalloc::PagemapConfig const**); using namespace snmalloc; int main() { + setup(); + MemoryProviderStateMixin mp; size_t size = 1ULL << 28; diff --git a/src/test/perf/contention/contention.cc b/src/test/perf/contention/contention.cc index 1858724..b53ec12 100644 --- a/src/test/perf/contention/contention.cc +++ b/src/test/perf/contention/contention.cc @@ -1,5 +1,6 @@ #include "test/measuretime.h" #include "test/opt.h" +#include "test/setup.h" #include "test/usage.h" #include "test/xoroshiro.h" @@ -148,6 +149,8 @@ void test_tasks(size_t num_tasks, size_t count, size_t size) int main(int argc, char** argv) { + setup(); + opt::Opt opt(argc, argv); size_t cores = opt.is("--cores", 8); diff --git a/src/test/perf/external_pointer/externalpointer.cc b/src/test/perf/external_pointer/externalpointer.cc index d87baeb..d4124b3 100644 --- a/src/test/perf/external_pointer/externalpointer.cc +++ b/src/test/perf/external_pointer/externalpointer.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -78,6 +79,8 @@ namespace test int main(int, char**) { + setup(); + xoroshiro::p128r64 r; #if NDEBUG size_t nn = 30; diff --git a/src/test/perf/singlethread/singlethread.cc b/src/test/perf/singlethread/singlethread.cc index 3b3700c..3909521 100644 --- a/src/test/perf/singlethread/singlethread.cc +++ b/src/test/perf/singlethread/singlethread.cc @@ -1,5 +1,6 @@ #include #include +#include #include using namespace snmalloc; @@ -63,6 +64,8 @@ void test_alloc_dealloc(size_t count, size_t size, bool write) int main(int, char**) { + setup(); + for (size_t size = 16; size <= 128; size <<= 1) { test_alloc_dealloc(1 << 15, size, false); diff --git a/src/test/setup.h b/src/test/setup.h new file mode 100644 index 0000000..528d5c0 --- /dev/null +++ b/src/test/setup.h @@ -0,0 +1,20 @@ +#if defined(WIN32) && defined(SNMALLOC_CI_BUILD) +# include +# include +# include +void _cdecl error(int signal) +{ + UNUSED(signal); + puts("*****ABORT******"); + _exit(1); +} +void setup() +{ + // Disable abort dialog box in CI builds. + _set_error_mode(_OUT_TO_STDERR); + _set_abort_behavior(0, _WRITE_ABORT_MSG); + signal(SIGABRT, error); +} +#else +void setup() {} +#endif From e9432fe9cac580222b69a973dca62267c507b37d Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 13 Aug 2019 16:31:41 +0100 Subject: [PATCH 11/11] Shrink reserve as failing in CI. --- src/test/func/two_alloc_types/main.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/func/two_alloc_types/main.cc b/src/test/func/two_alloc_types/main.cc index 9802b7c..b8ee2ff 100644 --- a/src/test/func/two_alloc_types/main.cc +++ b/src/test/func/two_alloc_types/main.cc @@ -45,7 +45,7 @@ int main() MemoryProviderStateMixin mp; - size_t size = 1ULL << 28; + size_t size = 1ULL << 26; oe_base = mp.reserve(&size, 1); oe_end = (uint8_t*)oe_base + size; std::cout << "Allocated region " << oe_base << " - " << oe_end << std::endl;