Skip to content

Commit 50f7400

Browse files
authored
GH-50083: [C++] Access mimalloc through dynamically-resolved symbols (#41128)
### Rationale for this change The [memray memory profiler](https://github.com/bloomberg/memray) works by interposing certain dynamic symbols in the profiled process to replace them with their own functions that will collect memory allocation data. It will currently, to the best of my knowledge, only recognize system C calls such `malloc`, `mmap`... When a third-party allocator like mimalloc or jemalloc is being used, such that Arrow does by default, memray does not see the logical allocation calls made through these allocator's APIs (because they are not interposed), but only the raw memory reservations that they issue using system routines. This can lead people using memray to think that a given Arrow workload (or any workload using such allocators, really) that an inordinate amount of memory is being used, while the reported memory mostly represents non-committed virtual memory that the allocator keeps for performance reasons. Concrete example in GH-40301: we allocate a number of 1kiB buffers from mimalloc, but memray sees a similar number of 64MiB calls to `mmap`. We [discussed](bloomberg/memray#577) how to enhance memray such as to account for the corresponding logical allocations, and we came to the conclusion that it requires that Arrow exposes API calls that can be dynamically interposed. Since we typically build against a static `libmimalloc.a`, the mimalloc symbols cannot be exposed (at least, I cannot seem to get this to work on Ubuntu). This means we need to define our own symbols wrapping the mimalloc APIs. ### What changes are included in this PR? Define public, interposable symbols that redirect into the mimalloc APIs that we use. ### Are these changes tested? Not for now. We could probably test them, at least on Linux, by compiling an almost trivial shared library and interposing it using `LD_PRELOAD`. ### Are there any user-facing changes? No. There should not be any noticeable performance regression, except perhaps on memory pool micro-benchmarks. * GitHub Issue: #50083 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 55958e0 commit 50f7400

7 files changed

Lines changed: 93 additions & 21 deletions

File tree

cpp/src/arrow/CMakeLists.txt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,24 @@ if(ARROW_JEMALLOC)
459459
set_source_files_properties(memory_pool_jemalloc.cc
460460
PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
461461
endif()
462+
if(ARROW_MIMALLOC)
463+
list(APPEND ARROW_MEMORY_POOL_SRCS memory_pool_mimalloc.cc)
464+
set_source_files_properties(memory_pool_mimalloc.cc
465+
PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
466+
# GH-50083: make sure some allocation-related public symbols are interposable,
467+
# for the benefit of memory profilers and other runtime analyzers.
468+
# Ideally we would scope a specific subset of symbols, but it doesn't seem
469+
# easily doable. See `memory_pool_internal.h`.
470+
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU"
471+
OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang"
472+
OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
473+
set_source_files_properties(${ARROW_MEMORY_POOL_SRCS}
474+
PROPERTIES COMPILE_OPTIONS "-fsemantic-interposition")
475+
endif()
476+
endif()
477+
462478
arrow_add_object_library(ARROW_MEMORY_POOL ${ARROW_MEMORY_POOL_SRCS})
479+
463480
if(ARROW_JEMALLOC)
464481
foreach(ARROW_MEMORY_POOL_TARGET ${ARROW_MEMORY_POOL_TARGETS})
465482
target_link_libraries(${ARROW_MEMORY_POOL_TARGET} PRIVATE jemalloc::jemalloc)

cpp/src/arrow/memory_pool.cc

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,11 @@
5454
#endif
5555

5656
namespace arrow {
57-
58-
namespace memory_pool {
59-
60-
namespace internal {
57+
namespace memory_pool::internal {
6158

6259
alignas(kDefaultBufferAlignment) int64_t zero_size_area[1] = {kDebugXorSuffix};
6360

64-
} // namespace internal
65-
66-
} // namespace memory_pool
61+
} // namespace memory_pool::internal
6762

6863
namespace {
6964

@@ -400,15 +395,15 @@ class MimallocAllocator {
400395
*out = memory_pool::internal::kZeroSizeArea;
401396
return Status::OK();
402397
}
403-
*out = reinterpret_cast<uint8_t*>(
404-
mi_malloc_aligned(static_cast<size_t>(size), static_cast<size_t>(alignment)));
398+
*out = reinterpret_cast<uint8_t*>(arrow_mi_malloc_aligned(
399+
static_cast<size_t>(size), static_cast<size_t>(alignment)));
405400
if (*out == NULL) {
406401
return Status::OutOfMemory("malloc of size ", size, " failed");
407402
}
408403
return Status::OK();
409404
}
410405

411-
static void ReleaseUnused() { mi_collect(true); }
406+
static void ReleaseUnused() { arrow_mi_collect(true); }
412407

413408
static Status ReallocateAligned(int64_t old_size, int64_t new_size, int64_t alignment,
414409
uint8_t** ptr) {
@@ -423,7 +418,7 @@ class MimallocAllocator {
423418
return Status::OK();
424419
}
425420
*ptr = reinterpret_cast<uint8_t*>(
426-
mi_realloc_aligned(previous_ptr, static_cast<size_t>(new_size), alignment));
421+
arrow_mi_realloc_aligned(previous_ptr, static_cast<size_t>(new_size), alignment));
427422
if (*ptr == NULL) {
428423
*ptr = previous_ptr;
429424
return Status::OutOfMemory("realloc of size ", new_size, " failed");
@@ -435,7 +430,7 @@ class MimallocAllocator {
435430
if (ptr == memory_pool::internal::kZeroSizeArea) {
436431
DCHECK_EQ(size, 0);
437432
} else {
438-
mi_free(ptr);
433+
arrow_mi_free(ptr);
439434
}
440435
}
441436

cpp/src/arrow/memory_pool_benchmark.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "arrow/memory_pool.h"
1919
#include "arrow/result.h"
20+
#include "arrow/util/config.h"
2021
#include "arrow/util/logging.h"
2122

2223
#include "benchmark/benchmark.h"

cpp/src/arrow/memory_pool_internal.h

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,9 @@
1919

2020
#include "arrow/memory_pool.h"
2121
#include "arrow/util/config.h"
22+
#include "arrow/util/macros.h"
2223

23-
namespace arrow {
24-
25-
namespace memory_pool {
26-
27-
namespace internal {
24+
namespace arrow::memory_pool::internal {
2825

2926
static constexpr int64_t kDebugXorSuffix = -0x181fe80e0b464188LL;
3027

@@ -49,8 +46,21 @@ class JemallocAllocator {
4946

5047
#endif // defined(ARROW_JEMALLOC)
5148

52-
} // namespace internal
49+
} // namespace arrow::memory_pool::internal
50+
51+
#ifdef ARROW_MIMALLOC
5352

54-
} // namespace memory_pool
53+
extern "C" {
54+
// GH-50083: expose public symbols with well-known names for memory profilers
55+
// to be able to intercept our mimalloc calls and better make sense of Arrow's
56+
// memory profile.
57+
// These symbols need to be interposable (using e.g. `LD_PRELOAD`), which is
58+
// ensured using `-fsemantic-interposition` in CMakeLists.txt.
59+
ARROW_EXPORT ARROW_NOINLINE void* arrow_mi_malloc_aligned(size_t size, size_t alignment);
60+
ARROW_EXPORT ARROW_NOINLINE void* arrow_mi_realloc_aligned(void* p, size_t new_size,
61+
size_t alignment);
62+
ARROW_EXPORT ARROW_NOINLINE void arrow_mi_free(void* p);
63+
ARROW_EXPORT ARROW_NOINLINE void arrow_mi_collect(bool force);
64+
}
5565

56-
} // namespace arrow
66+
#endif // defined(ARROW_MIMALLOC)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#include "arrow/memory_pool_internal.h"
19+
#include "arrow/util/io_util.h"
20+
#include "arrow/util/logging.h" // IWYU pragma: keep
21+
22+
#include <mimalloc.h>
23+
24+
// extern "C" {
25+
26+
void arrow_mi_free(void* p) { mi_free(p); }
27+
28+
void* arrow_mi_malloc_aligned(size_t size, size_t alignment) {
29+
return mi_malloc_aligned(size, alignment);
30+
}
31+
32+
void* arrow_mi_realloc_aligned(void* p, size_t new_size, size_t alignment) {
33+
return mi_realloc_aligned(p, new_size, alignment);
34+
}
35+
36+
void arrow_mi_collect(bool force) { mi_collect(force); }
37+
38+
// }

cpp/src/arrow/memory_pool_test.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,10 @@ TYPED_TEST_P(TestMemoryPool, Reallocate) { this->TestReallocate(); }
7878

7979
TYPED_TEST_P(TestMemoryPool, Alignment) { this->TestAlignment(); }
8080

81-
REGISTER_TYPED_TEST_SUITE_P(TestMemoryPool, MemoryTracking, OOM, Reallocate, Alignment);
81+
TYPED_TEST_P(TestMemoryPool, ReleaseUnused) { this->TestReleaseUnused(); }
82+
83+
REGISTER_TYPED_TEST_SUITE_P(TestMemoryPool, MemoryTracking, OOM, Reallocate, Alignment,
84+
ReleaseUnused);
8285

8386
INSTANTIATE_TYPED_TEST_SUITE_P(Default, TestMemoryPool, DefaultMemoryPoolFactory);
8487
INSTANTIATE_TYPED_TEST_SUITE_P(System, TestMemoryPool, SystemMemoryPoolFactory);

cpp/src/arrow/memory_pool_test.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ class TestMemoryPoolBase : public ::testing::Test {
106106
pool->Free(data512, 10, 512);
107107
}
108108
}
109+
110+
void TestReleaseUnused() {
111+
auto pool = memory_pool();
112+
const int64_t nbytes = pool->bytes_allocated();
113+
pool->ReleaseUnused();
114+
// Unfortunately there's not much that we can assert here
115+
ASSERT_EQ(nbytes, pool->bytes_allocated());
116+
}
109117
};
110118

111119
} // namespace arrow

0 commit comments

Comments
 (0)