Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions velox/common/memory/MmapAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@
#include "velox/common/memory/MmapAllocator.h"

#include <sys/mman.h>
#include <unistd.h>

#include "velox/common/base/Counters.h"
#include "velox/common/base/Portability.h"
#include "velox/common/base/StatsReporter.h"
#include "velox/common/memory/Memory.h"

namespace facebook::velox::memory {
bool MmapAllocator::isPageSizeSupported() {
return static_cast<uint64_t>(sysconf(_SC_PAGESIZE)) ==
AllocationTraits::kPageSize;
}

MmapAllocator::MmapAllocator(const Options& options)
: MemoryAllocator(options.largestSizeClass),
kind_(MemoryAllocator::Kind::kMmap),
Expand All @@ -39,6 +45,24 @@ MmapAllocator::MmapAllocator(const Options& options)
AllocationTraits::numPages(
options.capacity - mallocReservedBytes_),
64 * sizeClassSizes_.back())) {
// MmapAllocator tracks memory at AllocationTraits::kPageSize granularity
// and calls madvise() on individual pages, which requires the address and
// length to line up with the OS's actual page size. NVIDIA's documented
// recommended default page size for Grace / Grace-Hopper systems is 64KB,
// not 4KB (see
// https://docs.nvidia.com/dccpu/grace-perf-tuning-guide/os-settings.html),
// and on such systems madvise() silently fails with EINVAL on sub-64KB
// regions instead of throwing, which corrupts this allocator's internal
// page-count accounting rather than surfacing a clear error. This isn't
// Velox-specific: jemalloc has the identical >4KB-page limitation (see
// https://github.com/arangodb/arangodb/issues/22177). Fail fast here
// instead of silently corrupting state.
VELOX_CHECK(
isPageSizeSupported(),
"MmapAllocator requires the system page size to match AllocationTraits::kPageSize ({} bytes); system page size is {} bytes. Use MemoryAllocator::Kind::kMalloc on this system instead.",
AllocationTraits::kPageSize,
sysconf(_SC_PAGESIZE));

for (const auto& size : sizeClassSizes_) {
sizeClasses_.push_back(std::make_unique<SizeClass>(capacity_ / size, size));
}
Expand Down
9 changes: 9 additions & 0 deletions velox/common/memory/MmapAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ class MmapAllocator : public MemoryAllocator {

~MmapAllocator();

/// Returns true if the system's page size matches
/// AllocationTraits::kPageSize, the granularity MmapAllocator assumes for
/// its mmap/madvise calls (see the constructor in MmapAllocator.cpp for
/// why this can be false). Construct a MallocAllocator instead when it
/// is; the constructor enforces this via VELOX_CHECK, so callers that
/// want to select an allocator kind without throwing should check this
/// first.
static bool isPageSizeSupported();

Kind kind() const override {
return kind_;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,13 @@ class AbfsFileSystemTest : public testing::Test {
}

void TearDown() override {
azuriteServer_->stop();
// azuriteServer_ is left null if SetUp() threw before it could be
// constructed (e.g. the azurite-blob executable wasn't found).
// TearDown() runs unconditionally after SetUp(), even on failure, so
// it must not assume construction succeeded.
if (azuriteServer_ != nullptr) {
azuriteServer_->stop();
}
}

static std::string generateRandomData(int size) {
Expand Down
Loading