From 2404f8b9ec58ecee20993a12c9b463b7767295f1 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Wed, 28 Oct 2020 13:45:21 -0700 Subject: [PATCH] slightly improve jemalloc allocator API header (#7592) Summary: Fix a few typos and avoid a potential nullptr dereference. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7592 Reviewed By: zhichao-cao Differential Revision: D24582111 Pulled By: riversand963 fbshipit-source-id: 51e9260e8cad1fcdedd310c889f0faeec6efd937 --- include/rocksdb/memory_allocator.h | 24 ++++++++++++------------ memory/jemalloc_nodump_allocator.cc | 6 +++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/rocksdb/memory_allocator.h b/include/rocksdb/memory_allocator.h index 60256a977..51442239a 100644 --- a/include/rocksdb/memory_allocator.h +++ b/include/rocksdb/memory_allocator.h @@ -45,31 +45,31 @@ struct JemallocAllocatorOptions { bool limit_tcache_size = false; // Lower bound of allocation size to use tcache, if limit_tcache_size=true. - // When used with block cache, it is recommneded to set it to block_size/4. + // When used with block cache, it is recommended to set it to block_size/4. size_t tcache_size_lower_bound = 1024; // Upper bound of allocation size to use tcache, if limit_tcache_size=true. - // When used with block cache, it is recommneded to set it to block_size. + // When used with block cache, it is recommended to set it to block_size. size_t tcache_size_upper_bound = 16 * 1024; }; -// Generate memory allocators which allocates through Jemalloc and utilize -// MADV_DONTDUMP through madvice to exclude cache items from core dump. +// Generate memory allocator which allocates through Jemalloc and utilize +// MADV_DONTDUMP through madvise to exclude cache items from core dump. // Applications can use the allocator with block cache to exclude block cache // usage from core dump. // // Implementation details: -// The JemallocNodumpAllocator creates a delicated jemalloc arena, and all -// allocations of the JemallocNodumpAllocator is through the same arena. -// The memory allocator hooks memory allocation of the arena, and call -// madvice() with MADV_DONTDUMP flag to exclude the piece of memory from -// core dump. Side benefit of using single arena would be reduce of jemalloc -// metadata for some workload. +// The JemallocNodumpAllocator creates a dedicated jemalloc arena, and all +// allocations of the JemallocNodumpAllocator are through the same arena. +// The memory allocator hooks memory allocation of the arena, and calls +// madvise() with MADV_DONTDUMP flag to exclude the piece of memory from +// core dump. Side benefit of using single arena would be reduction of jemalloc +// metadata for some workloads. // // To mitigate mutex contention for using one single arena, jemalloc tcache // (thread-local cache) is enabled to cache unused allocations for future use. -// The tcache normally incur 0.5M extra memory usage per-thread. The usage -// can be reduce by limitting allocation sizes to cache. +// The tcache normally incurs 0.5M extra memory usage per-thread. The usage +// can be reduced by limiting allocation sizes to cache. extern Status NewJemallocNodumpAllocator( JemallocAllocatorOptions& options, std::shared_ptr* memory_allocator); diff --git a/memory/jemalloc_nodump_allocator.cc b/memory/jemalloc_nodump_allocator.cc index 980b08b95..a01034e3f 100644 --- a/memory/jemalloc_nodump_allocator.cc +++ b/memory/jemalloc_nodump_allocator.cc @@ -132,6 +132,9 @@ size_t JemallocNodumpAllocator::UsableSize(void* p, Status NewJemallocNodumpAllocator( JemallocAllocatorOptions& options, std::shared_ptr* memory_allocator) { + if (memory_allocator == nullptr) { + return Status::InvalidArgument("memory_allocator must be non-null."); + } *memory_allocator = nullptr; Status unsupported = Status::NotSupported( "JemallocNodumpAllocator only available with jemalloc version >= 5 " @@ -143,9 +146,6 @@ Status NewJemallocNodumpAllocator( if (!HasJemalloc()) { return unsupported; } - if (memory_allocator == nullptr) { - return Status::InvalidArgument("memory_allocator must be non-null."); - } if (options.limit_tcache_size && options.tcache_size_lower_bound >= options.tcache_size_upper_bound) { return Status::InvalidArgument(