From c051534e3e17f945634dac5691da50771f6126ed Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 9 Feb 2022 14:03:42 +0100 Subject: Revert "Revert "Wire in mallopt(in param, int value) interface in vespamalloc and ver…"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- vespamalloc/src/tests/stacktrace/stacktrace.cpp | 1 - vespamalloc/src/tests/test1/new_test.cpp | 27 ++++++++++++++++++++--- vespamalloc/src/vespamalloc/malloc/malloc.h | 8 ++++++- vespamalloc/src/vespamalloc/malloc/overload.h | 7 +++++- vespamalloc/src/vespamalloc/malloc/threadpool.h | 2 ++ vespamalloc/src/vespamalloc/malloc/threadpool.hpp | 11 +++++++++ 6 files changed, 50 insertions(+), 6 deletions(-) diff --git a/vespamalloc/src/tests/stacktrace/stacktrace.cpp b/vespamalloc/src/tests/stacktrace/stacktrace.cpp index b28a9653d27..2f0d2eb2277 100644 --- a/vespamalloc/src/tests/stacktrace/stacktrace.cpp +++ b/vespamalloc/src/tests/stacktrace/stacktrace.cpp @@ -1,6 +1,5 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include -#include #include #include #include diff --git a/vespamalloc/src/tests/test1/new_test.cpp b/vespamalloc/src/tests/test1/new_test.cpp index 5230869145d..6f84f7c0f63 100644 --- a/vespamalloc/src/tests/test1/new_test.cpp +++ b/vespamalloc/src/tests/test1/new_test.cpp @@ -3,7 +3,6 @@ #include #include #include -#include LOG_SETUP("new_test"); @@ -134,14 +133,28 @@ void verify_vespamalloc_usable_size() { } } +enum class MallocLibrary { UNKNOWN, VESPA_MALLOC, VESPA_MALLOC_D}; + +MallocLibrary +detectLibrary() { + if (dlsym(RTLD_NEXT, "is_vespamallocd") != nullptr) { + // Debug variants will never have more memory available as there is pre/postamble for error detection. + return MallocLibrary::VESPA_MALLOC_D; + } else if (dlsym(RTLD_NEXT, "is_vespamalloc") != nullptr) { + return MallocLibrary::VESPA_MALLOC; + } + return MallocLibrary::UNKNOWN; +} + TEST("verify malloc_usable_size is sane") { constexpr size_t SZ = 33; std::unique_ptr buf = std::make_unique(SZ); size_t usable_size = malloc_usable_size(buf.get()); - if (dlsym(RTLD_NEXT, "is_vespamallocd") != nullptr) { + MallocLibrary env = detectLibrary(); + if (env == MallocLibrary::VESPA_MALLOC_D) { // Debug variants will never have more memory available as there is pre/postamble for error detection. EXPECT_EQUAL(SZ, usable_size); - } else if (dlsym(RTLD_NEXT, "is_vespamalloc") != nullptr) { + } else if (env == MallocLibrary::VESPA_MALLOC) { // Normal production vespamalloc will round up EXPECT_EQUAL(64u, usable_size); verify_vespamalloc_usable_size(); @@ -151,5 +164,13 @@ TEST("verify malloc_usable_size is sane") { } } +TEST("verify mallopt") { + MallocLibrary env = detectLibrary(); + if (env == MallocLibrary::UNKNOWN) return; + EXPECT_EQUAL(0, mallopt(M_MMAP_MAX, 0x1000000)); + EXPECT_EQUAL(1, mallopt(M_MMAP_THRESHOLD, 0x1000000)); + EXPECT_EQUAL(1, mallopt(M_MMAP_THRESHOLD, -1)); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespamalloc/src/vespamalloc/malloc/malloc.h b/vespamalloc/src/vespamalloc/malloc/malloc.h index 8e0a39e944b..449dce29896 100644 --- a/vespamalloc/src/vespamalloc/malloc/malloc.h +++ b/vespamalloc/src/vespamalloc/malloc/malloc.h @@ -25,6 +25,7 @@ public: } size_t getMaxNumThreads() const override { return _threadList.getMaxNumThreads(); } + int mallopt(int param, int value); void *malloc(size_t sz); void *malloc(size_t sz, std::align_val_t); void *realloc(void *oldPtr, size_t sz); @@ -148,6 +149,11 @@ void MemoryManager::info(FILE * os, size_t level) fflush(os); } +template +int MemoryManager::mallopt(int param, int value) { + return _threadList.getCurrent().mallopt(param, value); +} + template void * MemoryManager::malloc(size_t sz) { @@ -205,7 +211,7 @@ void MemoryManager::freeSC(void *ptr, SizeClassT sc) template void * MemoryManager::realloc(void *oldPtr, size_t sz) { - void *ptr(NULL); + void *ptr(nullptr); if (oldPtr) { MemBlockPtrT mem(oldPtr); mem.readjustAlignment(_segment); diff --git a/vespamalloc/src/vespamalloc/malloc/overload.h b/vespamalloc/src/vespamalloc/malloc/overload.h index 69d95ef5cdc..7d9c2b9c72e 100644 --- a/vespamalloc/src/vespamalloc/malloc/overload.h +++ b/vespamalloc/src/vespamalloc/malloc/overload.h @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include class CreateAllocator @@ -140,6 +140,11 @@ struct mallinfo mallinfo() __THROW { } #endif +int mallopt(int param, int value) throw() __attribute((visibility("default"))); +int mallopt(int param, int value) throw() { + return vespamalloc::createAllocator()->mallopt(param, value); +} + void * malloc(size_t sz) __attribute((visibility("default"))); void * malloc(size_t sz) { return vespamalloc::createAllocator()->malloc(sz); diff --git a/vespamalloc/src/vespamalloc/malloc/threadpool.h b/vespamalloc/src/vespamalloc/malloc/threadpool.h index ac4f5bb8551..ec89079a415 100644 --- a/vespamalloc/src/vespamalloc/malloc/threadpool.h +++ b/vespamalloc/src/vespamalloc/malloc/threadpool.h @@ -19,6 +19,7 @@ public: void setPool(AllocPool & pool) { _allocPool = & pool; } + int mallopt(int param, int value); void malloc(size_t sz, MemBlockPtrT & mem); void free(MemBlockPtrT mem, SizeClassT sc); @@ -65,6 +66,7 @@ private: static constexpr bool alwaysReuse(SizeClassT sc) { return sc > ALWAYS_REUSE_SC_LIMIT; } AllocPool * _allocPool; + ssize_t _mmapLimit; AllocFree _memList[NUM_SIZE_CLASSES]; ThreadStatT _stat[NUM_SIZE_CLASSES]; uint32_t _threadId; diff --git a/vespamalloc/src/vespamalloc/malloc/threadpool.hpp b/vespamalloc/src/vespamalloc/malloc/threadpool.hpp index 7e86c3f691a..a3691a496dd 100644 --- a/vespamalloc/src/vespamalloc/malloc/threadpool.hpp +++ b/vespamalloc/src/vespamalloc/malloc/threadpool.hpp @@ -2,6 +2,7 @@ #pragma once #include +#include namespace vespamalloc { @@ -85,6 +86,7 @@ mallocHelper(size_t exactSize, template ThreadPoolT::ThreadPoolT() : _allocPool(nullptr), + _mmapLimit(-1), _threadId(0), _osThreadId(0) { @@ -93,6 +95,15 @@ ThreadPoolT::ThreadPoolT() : template ThreadPoolT::~ThreadPoolT() = default; +template +int ThreadPoolT::mallopt(int param, int value) { + if (param == M_MMAP_THRESHOLD) { + _mmapLimit = value; + return 1; + } + return 0; +} + template void ThreadPoolT::malloc(size_t sz, MemBlockPtrT & mem) { -- cgit v1.2.3 From 1a67c21b53b747b1aca81e8b942238583234c985 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 9 Feb 2022 13:06:03 +0000 Subject: Add back header --- vespamalloc/src/tests/test1/new_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/vespamalloc/src/tests/test1/new_test.cpp b/vespamalloc/src/tests/test1/new_test.cpp index 6f84f7c0f63..0723f8cca85 100644 --- a/vespamalloc/src/tests/test1/new_test.cpp +++ b/vespamalloc/src/tests/test1/new_test.cpp @@ -3,6 +3,7 @@ #include #include #include +#include LOG_SETUP("new_test"); -- cgit v1.2.3 From 3903e12659b57760a8ffe3140747daa13d01068d Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 9 Feb 2022 13:37:59 +0000 Subject: Add a simple MallocMmapGuard --- .../src/tests/shutdownguard/shutdownguard_test.cpp | 14 ++++++++------ staging_vespalib/src/vespa/vespalib/util/CMakeLists.txt | 3 ++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp b/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp index 79777cdd53f..0fb2dfdda4b 100644 --- a/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp +++ b/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include #include +#include #include #include #include @@ -8,12 +9,8 @@ using namespace vespalib; -TEST_SETUP(Test); - -int -Test::Main() +TEST("test shutdown guard") { - TEST_INIT("shutdownguard_test"); { ShutdownGuard farFuture(1000000s); std::this_thread::sleep_for(20ms); @@ -37,5 +34,10 @@ Test::Main() } EXPECT_TRUE(i < 800); } - TEST_DONE(); } + +TEST("test malloc mmap guard") { + MallocMmapGuard guard(0x100000); +} + +TEST_MAIN() { TEST_RUN_ALL(); } \ No newline at end of file diff --git a/staging_vespalib/src/vespa/vespalib/util/CMakeLists.txt b/staging_vespalib/src/vespa/vespalib/util/CMakeLists.txt index 2b52e9e167f..e69dd36d6f5 100644 --- a/staging_vespalib/src/vespa/vespalib/util/CMakeLists.txt +++ b/staging_vespalib/src/vespa/vespalib/util/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(staging_vespalib_vespalib_util OBJECT bits.cpp clock.cpp crc.cpp + document_runnable.cpp doom.cpp foregroundtaskexecutor.cpp growablebytebuffer.cpp @@ -12,10 +13,10 @@ vespa_add_library(staging_vespalib_vespalib_util OBJECT jsonexception.cpp jsonstream.cpp jsonwriter.cpp + malloc_mmap_guard.cpp process_memory_stats.cpp programoptions.cpp programoptions_testutils.cpp - document_runnable.cpp rusage.cpp sequencedtaskexecutor.cpp sequencedtaskexecutorobserver.cpp -- cgit v1.2.3 From 3df91301de5e6eb415174a6e5cd0786b8927ba47 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 9 Feb 2022 13:50:33 +0000 Subject: Add a simple MallocMmapGuard --- .../src/vespa/vespalib/util/malloc_mmap_guard.cpp | 22 ++++++++++++++++++ .../src/vespa/vespalib/util/malloc_mmap_guard.h | 27 ++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp create mode 100644 staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h diff --git a/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp new file mode 100644 index 00000000000..2b932981823 --- /dev/null +++ b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp @@ -0,0 +1,22 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "malloc_mmap_guard.h" +#include +#include +#include + +namespace vespalib { + +MallocMmapGuard::MallocMmapGuard(size_t mmapLimit) : + _threadId(std::this_thread::get_id()) +{ + int limit = mmapLimit <= std::numeric_limits::max() ? mmapLimit : std::numeric_limits::max(); + mallopt(M_MMAP_THRESHOLD, limit); +} + +MallocMmapGuard::~MallocMmapGuard() +{ + assert(_threadId == std::this_thread::get_id()); + mallopt(M_MMAP_THRESHOLD, -1); +} + +} diff --git a/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h new file mode 100644 index 00000000000..3cf26202f9b --- /dev/null +++ b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h @@ -0,0 +1,27 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include + +namespace vespalib { + +/** + * Provides a hint to malloc implementation that all allocations in the scope of this guard + * will use mmap directly for allocation larger than the givem limit. + * The effect is implementation dependent. vespamalloc applies this only for the calling thread. + **/ +class MallocMmapGuard +{ +public: + MallocMmapGuard(size_t mmapLimit); + MallocMmapGuard(const MallocMmapGuard &) = delete; + MallocMmapGuard & operator=(const MallocMmapGuard &) = delete; + MallocMmapGuard(MallocMmapGuard &&) = delete; + MallocMmapGuard & operator=(MallocMmapGuard &&) = delete; + ~MallocMmapGuard(); +private: + std::thread::id _threadId; +}; + +} // namespace vespalib + -- cgit v1.2.3 From 5f96af6e4995a7a38445f3d24482baaedeec0668 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 9 Feb 2022 16:33:49 +0000 Subject: Let default limit be 1G, and add extra comment about usage and non-usage. --- staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp | 2 +- staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp | 3 ++- staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h | 5 +++-- vespamalloc/src/vespamalloc/malloc/threadpool.hpp | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp b/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp index 0fb2dfdda4b..348e9bbd503 100644 --- a/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp +++ b/staging_vespalib/src/tests/shutdownguard/shutdownguard_test.cpp @@ -40,4 +40,4 @@ TEST("test malloc mmap guard") { MallocMmapGuard guard(0x100000); } -TEST_MAIN() { TEST_RUN_ALL(); } \ No newline at end of file +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp index 2b932981823..0ced160fda2 100644 --- a/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.cpp @@ -1,5 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "malloc_mmap_guard.h" +#include #include #include #include @@ -16,7 +17,7 @@ MallocMmapGuard::MallocMmapGuard(size_t mmapLimit) : MallocMmapGuard::~MallocMmapGuard() { assert(_threadId == std::this_thread::get_id()); - mallopt(M_MMAP_THRESHOLD, -1); + mallopt(M_MMAP_THRESHOLD, 1_Gi); } } diff --git a/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h index 3cf26202f9b..03e6d38c03c 100644 --- a/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h +++ b/staging_vespalib/src/vespa/vespalib/util/malloc_mmap_guard.h @@ -7,7 +7,9 @@ namespace vespalib { /** * Provides a hint to malloc implementation that all allocations in the scope of this guard - * will use mmap directly for allocation larger than the givem limit. + * will use mmap directly for allocation larger than the given limit. + * NB !! Note that guards can not be nested. Intention is to use around third party libraries where + * you do not control allocation yourself. * The effect is implementation dependent. vespamalloc applies this only for the calling thread. **/ class MallocMmapGuard @@ -24,4 +26,3 @@ private: }; } // namespace vespalib - diff --git a/vespamalloc/src/vespamalloc/malloc/threadpool.hpp b/vespamalloc/src/vespamalloc/malloc/threadpool.hpp index a3691a496dd..e9b9fabebdc 100644 --- a/vespamalloc/src/vespamalloc/malloc/threadpool.hpp +++ b/vespamalloc/src/vespamalloc/malloc/threadpool.hpp @@ -86,7 +86,7 @@ mallocHelper(size_t exactSize, template ThreadPoolT::ThreadPoolT() : _allocPool(nullptr), - _mmapLimit(-1), + _mmapLimit(0x40000000), _threadId(0), _osThreadId(0) { -- cgit v1.2.3