From 339424f5ea0e7452e4b7160beddfdaac2e08f993 Mon Sep 17 00:00:00 2001 From: proller Date: Thu, 16 Nov 2017 14:27:31 +0300 Subject: [PATCH] Fix realloc on freebsd and macos --- CMakeLists.txt | 8 +++- dbms/src/Common/Allocator.cpp | 21 ++--------- libs/libcommon/CMakeLists.txt | 2 + libs/libcommon/cmake/find_gperftools.cmake | 8 +++- libs/libcommon/include/common/mremap.h | 44 ++++++++++++++++++++++ libs/libcommon/src/mremap.cpp | 40 ++++++++++++++++++++ 6 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 libs/libcommon/include/common/mremap.h create mode 100644 libs/libcommon/src/mremap.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 336b5da3524..4d20e398c09 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -170,7 +170,13 @@ if (NOT MAKE_STATIC_LIBRARIES) set(CMAKE_POSITION_INDEPENDENT_CODE ON) endif () -set (SAN_FLAGS "-O3 -g -fno-omit-frame-pointer") +set (SAN_FLAGS "${SAN_FLAGS} -g -fno-omit-frame-pointer") +if (SAN_DEBUG) + set (SAN_FLAGS "${SAN_FLAGS} -O0") +else () + set (SAN_FLAGS "${SAN_FLAGS} -O3") +endif () + set (CMAKE_CXX_FLAGS_ASAN "${CMAKE_CXX_FLAGS_ASAN} ${SAN_FLAGS} -fsanitize=address") set (CMAKE_C_FLAGS_ASAN "${CMAKE_C_FLAGS_ASAN} ${SAN_FLAGS} -fsanitize=address") set (CMAKE_CXX_FLAGS_UBSAN "${CMAKE_CXX_FLAGS_UBSAN} ${SAN_FLAGS} -fsanitize=undefined") diff --git a/dbms/src/Common/Allocator.cpp b/dbms/src/Common/Allocator.cpp index 8b3d0b65a94..31c68fa5257 100644 --- a/dbms/src/Common/Allocator.cpp +++ b/dbms/src/Common/Allocator.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -114,7 +115,6 @@ void Allocator::free(void * buf, size_t size) template void * Allocator::realloc(void * buf, size_t old_size, size_t new_size, size_t alignment) { -#if !defined(__APPLE__) && !defined(__FreeBSD__) if (old_size < MMAP_THRESHOLD && new_size < MMAP_THRESHOLD && alignment <= MALLOC_MIN_ALIGNMENT) { CurrentMemoryTracker::realloc(old_size, new_size); @@ -131,28 +131,13 @@ void * Allocator::realloc(void * buf, size_t old_size, size_t new { CurrentMemoryTracker::realloc(old_size, new_size); - buf = mremap(buf, old_size, new_size, MREMAP_MAYMOVE); + // On apple and freebsd self-implemented mremap used (common/mremap.h) + buf = clickhouse_mremap(buf, old_size, new_size, MREMAP_MAYMOVE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); if (MAP_FAILED == buf) DB::throwFromErrno("Allocator: Cannot mremap memory chunk from " + formatReadableSizeWithBinarySuffix(old_size) + " to " + formatReadableSizeWithBinarySuffix(new_size) + ".", DB::ErrorCodes::CANNOT_MREMAP); /// No need for zero-fill, because mmap guarantees it. } -#else - // TODO: We need to use mmap/calloc on Apple too. - if ((old_size < MMAP_THRESHOLD && new_size < MMAP_THRESHOLD && alignment <= MALLOC_MIN_ALIGNMENT) || - (old_size >= MMAP_THRESHOLD && new_size >= MMAP_THRESHOLD)) - { - CurrentMemoryTracker::realloc(old_size, new_size); - - buf = ::realloc(buf, new_size); - - if (nullptr == buf) - DB::throwFromErrno("Allocator: Cannot realloc from " + formatReadableSizeWithBinarySuffix(old_size) + " to " + formatReadableSizeWithBinarySuffix(new_size) + ".", DB::ErrorCodes::CANNOT_ALLOCATE_MEMORY); - - if (clear_memory) - memset(reinterpret_cast(buf) + old_size, 0, new_size - old_size); - } -#endif else { void * new_buf = alloc(new_size, alignment); diff --git a/libs/libcommon/CMakeLists.txt b/libs/libcommon/CMakeLists.txt index 3ba10c85636..742b2633f40 100644 --- a/libs/libcommon/CMakeLists.txt +++ b/libs/libcommon/CMakeLists.txt @@ -21,6 +21,7 @@ add_library (common src/DateLUT.cpp src/DateLUTImpl.cpp src/exp10.cpp + src/mremap.cpp src/JSON.cpp src/getMemoryAmount.cpp src/ThreadPool.cpp @@ -34,6 +35,7 @@ add_library (common include/common/LocalDateTime.h include/common/ErrorHandlers.h include/common/exp10.h + include/common/mremap.h include/common/likely.h include/common/logger_useful.h include/common/MultiVersion.h diff --git a/libs/libcommon/cmake/find_gperftools.cmake b/libs/libcommon/cmake/find_gperftools.cmake index bbec87d72f9..31c9373586c 100644 --- a/libs/libcommon/cmake/find_gperftools.cmake +++ b/libs/libcommon/cmake/find_gperftools.cmake @@ -3,7 +3,13 @@ if (CMAKE_SYSTEM MATCHES "FreeBSD" OR ARCH_32) else () option (USE_INTERNAL_GPERFTOOLS_LIBRARY "Set to FALSE to use system gperftools (tcmalloc) library instead of bundled" ${NOT_UNBUNDLED}) endif () -option (ENABLE_LIBTCMALLOC "Set to TRUE to enable libtcmalloc" ON) + +if (CMAKE_SYSTEM MATCHES "FreeBSD") + option (ENABLE_LIBTCMALLOC "Set to TRUE to enable libtcmalloc" OFF) +else () + option (ENABLE_LIBTCMALLOC "Set to TRUE to enable libtcmalloc" ON) +endif () + option (DEBUG_LIBTCMALLOC "Set to TRUE to use debug version of libtcmalloc" OFF) if (ENABLE_LIBTCMALLOC) diff --git a/libs/libcommon/include/common/mremap.h b/libs/libcommon/include/common/mremap.h new file mode 100644 index 00000000000..e8271589ac0 --- /dev/null +++ b/libs/libcommon/include/common/mremap.h @@ -0,0 +1,44 @@ +#pragma once + +#include +#include + +#if defined(MREMAP_MAYMOVE) +// we already have implementation (linux) +#else + +#define MREMAP_MAYMOVE 1 + +void * mremap(void * old_address, + size_t old_size, + size_t new_size, + int flags = 0, + int mmap_prot = 0, + int mmap_flags = 0, + int mmap_fd = -1, + off_t mmap_offset = 0); + +#endif + +inline void * clickhouse_mremap(void * old_address, + size_t old_size, + size_t new_size, + int flags = 0, + int mmap_prot = 0, + int mmap_flags = 0, + int mmap_fd = -1, + off_t mmap_offset = 0) +{ + return mremap(old_address, + old_size, + new_size, + flags +#if !defined(MREMAP_FIXED) + , + mmap_prot, + mmap_flags, + mmap_fd, + mmap_offset +#endif + ); +} diff --git a/libs/libcommon/src/mremap.cpp b/libs/libcommon/src/mremap.cpp new file mode 100644 index 00000000000..159cc602325 --- /dev/null +++ b/libs/libcommon/src/mremap.cpp @@ -0,0 +1,40 @@ + +#include +#include +#include +#include +#include + +#if defined(MREMAP_FIXED) +// we already have implementation (linux) +#else + +void * mremap( + void * old_address, size_t old_size, size_t new_size, int flags, int mmap_prot, int mmap_flags, int mmap_fd, off_t mmap_offset) +{ + /// No actual shrink + if (new_size < old_size) + return old_address; + + if (!(flags & MREMAP_MAYMOVE)) + { + return nullptr; + } + + void * new_address = mmap(nullptr, new_size, mmap_prot, mmap_flags, mmap_fd, mmap_offset); + if (MAP_FAILED == new_address) + { + return MAP_FAILED; + } + + memcpy(new_address, old_address, old_size); + + if (munmap(old_address, old_size)) + { + abort(); + } + + return new_address; +} + +#endif