Skip to content

Commit

Permalink
change static to global
Browse files Browse the repository at this point in the history
  • Loading branch information
majetideepak committed Dec 5, 2024
1 parent 888dd49 commit 5f03e30
Show file tree
Hide file tree
Showing 14 changed files with 24 additions and 28 deletions.
5 changes: 2 additions & 3 deletions velox/buffer/tests/BufferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,14 @@

#include "folly/Range.h"
#include "velox/common/base/tests/GTestUtils.h"
#include "velox/common/config/GlobalConfig.h"
#include "velox/common/testutil/TestValue.h"
#include "velox/type/StringView.h"

#include <glog/logging.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>

DECLARE_bool(velox_enable_memory_usage_track_in_default_memory_pool);

namespace facebook {
namespace velox {

Expand All @@ -39,7 +38,7 @@ static_assert(!Buffer::is_pod_like_v<std::shared_ptr<int>>, "");
class BufferTest : public testing::Test {
protected:
static void SetUpTestCase() {
FLAGS_velox_enable_memory_usage_track_in_default_memory_pool = true;
config::globalConfig.enableMemoryUsageTrackInDefaultMemoryPool = true;
}

void SetUp() override {
Expand Down
5 changes: 1 addition & 4 deletions velox/common/config/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@

namespace facebook::velox::config {

GlobalConfig& globalConfig() {
static GlobalConfig config;
return config;
}
GlobalConfiguration globalConfig;

double toBytesPerCapacityUnit(CapacityUnit unit) {
switch (unit) {
Expand Down
4 changes: 2 additions & 2 deletions velox/common/config/GlobalConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

namespace facebook::velox::config {

struct GlobalConfig {
struct GlobalConfiguration {
/// Number of shared leaf memory pools per process.
int32_t memoryNumSharedLeafPoolsConfig{32};
/// If true, check fails on any memory leaks in memory pool and memory
Expand All @@ -45,6 +45,6 @@ struct GlobalConfig {
bool memoryPoolCapacityTransferAcrossTasks{false};
};

GlobalConfig& globalConfig();
extern GlobalConfiguration globalConfig;

} // namespace facebook::velox::config
2 changes: 1 addition & 1 deletion velox/common/memory/MallocAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ MallocAllocator::MallocAllocator(size_t capacity, uint32_t reservationByteLimit)

MallocAllocator::~MallocAllocator() {
// TODO: Remove the check when memory leak issue is resolved.
if (config::GlobalConfig().memoryLeakCheckEnabled) {
if (config::globalConfig.memoryLeakCheckEnabled) {
VELOX_CHECK(
((allocatedBytes_ - reservations_.read()) == 0) &&
(numAllocated_ == 0) && (numMapped_ == 0),
Expand Down
4 changes: 2 additions & 2 deletions velox/common/memory/Memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ std::vector<std::shared_ptr<MemoryPool>> createSharedLeafMemoryPools(
VELOX_CHECK_EQ(sysPool.name(), kSysRootName);
std::vector<std::shared_ptr<MemoryPool>> leafPools;
const size_t numSharedPools =
std::max(1, config::GlobalConfig().memoryNumSharedLeafPoolsConfig);
std::max(1, config::globalConfig.memoryNumSharedLeafPoolsConfig);
leafPools.reserve(numSharedPools);
for (size_t i = 0; i < numSharedPools; ++i) {
leafPools.emplace_back(
Expand Down Expand Up @@ -128,7 +128,7 @@ MemoryManager::MemoryManager(const MemoryManagerOptions& options)
sysRoot_->name());
VELOX_CHECK_EQ(
sharedLeafPools_.size(),
std::max(1, config::GlobalConfig().memoryNumSharedLeafPoolsConfig));
std::max(1, config::globalConfig.memoryNumSharedLeafPoolsConfig));
}

MemoryManager::~MemoryManager() {
Expand Down
6 changes: 3 additions & 3 deletions velox/common/memory/Memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,18 @@ struct MemoryManagerOptions {

/// If true, enable memory usage tracking in the default memory pool.
bool trackDefaultUsage{
config::GlobalConfig().enableMemoryUsageTrackInDefaultMemoryPool};
config::globalConfig.enableMemoryUsageTrackInDefaultMemoryPool};

/// If true, check the memory pool and usage leaks on destruction.
///
/// TODO: deprecate this flag after all the existing memory leak use cases
/// have been fixed.
bool checkUsageLeak{config::GlobalConfig().memoryLeakCheckEnabled};
bool checkUsageLeak{config::globalConfig.memoryLeakCheckEnabled};

/// If true, the memory pool will be running in debug mode to track the
/// allocation and free call stacks to detect the source of memory leak for
/// testing purpose.
bool debugEnabled{config::GlobalConfig().memoryPoolDebugEnabled};
bool debugEnabled{config::globalConfig.memoryPoolDebugEnabled};

/// Terminates the process and generates a core file on an allocation failure
bool coreOnAllocationFailureEnabled{false};
Expand Down
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ void MemoryAllocator::useHugePages(
const ContiguousAllocation& data,
bool enable) {
#ifdef linux
if (config::GlobalConfig().memoryUseHugepages) {
if (config::globalConfig.memoryUseHugepages) {
return;
}
auto maybeRange = data.hugePageRange();
Expand Down
4 changes: 2 additions & 2 deletions velox/common/memory/MemoryAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct Stats {

template <typename Op>
void recordAllocate(int64_t bytes, int32_t count, Op op) {
if (config::GlobalConfig().timeAllocations) {
if (config::globalConfig.timeAllocations) {
auto index = sizeIndex(bytes);
velox::ClockTimer timer(sizes[index].allocateClocks);
op();
Expand All @@ -107,7 +107,7 @@ struct Stats {

template <typename Op>
void recordFree(int64_t bytes, Op op) {
if (config::globalConfig().timeAllocations) {
if (config::globalConfig.timeAllocations) {
auto index = sizeIndex(bytes);
ClockTimer timer(sizes[index].freeClocks);
op();
Expand Down
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ std::string MemoryPoolImpl::treeMemoryUsage(bool skipEmptyPool) const {
if (parent_ != nullptr) {
return parent_->treeMemoryUsage(skipEmptyPool);
}
if (config::globalConfig().suppressMemoryCapacityExceedingErrorMessage) {
if (config::globalConfig.suppressMemoryCapacityExceedingErrorMessage) {
return "";
}
std::stringstream out;
Expand Down
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class MemoryPool : public std::enable_shared_from_this<MemoryPool> {

/// If true, tracks the allocation and free call stacks to detect the source
/// of memory leak for testing purpose.
bool debugEnabled{config::globalConfig().memoryPoolDebugEnabled};
bool debugEnabled{config::globalConfig.memoryPoolDebugEnabled};

/// Terminates the process and generates a core file on an allocation
/// failure
Expand Down
2 changes: 1 addition & 1 deletion velox/common/memory/MmapAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ MachinePageCount MmapAllocator::freeNonContiguousInternal(
ClockTimer timer(clocks);
pages = sizeClass->free(allocation);
}
if ((pages > 0) && config::globalConfig().timeAllocations) {
if ((pages > 0) && config::globalConfig.timeAllocations) {
// Increment the free time only if the allocation contained
// pages in the class. Note that size class indices in the
// allocator are not necessarily the same as in the stats.
Expand Down
6 changes: 3 additions & 3 deletions velox/common/memory/tests/MemoryAllocatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class MemoryAllocatorTest : public testing::TestWithParam<int> {
protected:
static void SetUpTestCase() {
TestValue::enable();
config::globalConfig().memoryLeakCheckEnabled = true;
config::globalConfig.memoryLeakCheckEnabled = true;
}

void SetUp() override {
Expand Down Expand Up @@ -650,7 +650,7 @@ TEST_P(MemoryAllocatorTest, stats) {
}

gflags::FlagSaver flagSaver;
config::globalConfig().timeAllocations = true;
config::globalConfig.timeAllocations = true;
for (auto i = 0; i < sizes.size(); ++i) {
std::unique_ptr<Allocation> allocation = std::make_unique<Allocation>();
auto size = sizes[i];
Expand All @@ -669,7 +669,7 @@ TEST_P(MemoryAllocatorTest, singleAllocation) {
return;
}
gflags::FlagSaver flagSaver;
config::globalConfig().timeAllocations = true;
config::globalConfig.timeAllocations = true;
const std::vector<MachinePageCount>& sizes = instance_->sizeClasses();
MachinePageCount capacity = kCapacityPages;
for (auto i = 0; i < sizes.size(); ++i) {
Expand Down
4 changes: 2 additions & 2 deletions velox/exec/Operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ void Operator::MemoryReclaimer::enterArbitration() {
}

Driver* const runningDriver = driverThreadCtx->driverCtx()->driver;
if (!config::globalConfig().memoryPoolCapacityTransferAcrossTasks) {
if (!config::globalConfig.memoryPoolCapacityTransferAcrossTasks) {
if (auto opDriver = ensureDriver()) {
// NOTE: the current running driver might not be the driver of the
// operator that requests memory arbitration. The reason is that an
Expand Down Expand Up @@ -680,7 +680,7 @@ void Operator::MemoryReclaimer::leaveArbitration() noexcept {
return;
}
Driver* const runningDriver = driverThreadCtx->driverCtx()->driver;
if (!config::globalConfig().memoryPoolCapacityTransferAcrossTasks) {
if (!config::globalConfig.memoryPoolCapacityTransferAcrossTasks) {
if (auto opDriver = ensureDriver()) {
VELOX_CHECK_EQ(
runningDriver->task()->taskId(),
Expand Down
4 changes: 2 additions & 2 deletions velox/exec/fuzzer/FuzzerUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ void setupMemory(
int64_t allocatorCapacity,
int64_t arbitratorCapacity,
bool enableGlobalArbitration) {
config::globalConfig().enableMemoryUsageTrackInDefaultMemoryPool = true;
config::globalConfig().memoryLeakCheckEnabled = true;
config::globalConfig.enableMemoryUsageTrackInDefaultMemoryPool = true;
config::globalConfig.memoryLeakCheckEnabled = true;
facebook::velox::memory::SharedArbitrator::registerFactory();
facebook::velox::memory::MemoryManagerOptions options;
options.allocatorCapacity = allocatorCapacity;
Expand Down

0 comments on commit 5f03e30

Please sign in to comment.