This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 72ffc8b3f [util] modernize MemTracker
72ffc8b3f is described below
commit 72ffc8b3fce9ba2293dcb1264a06196ce8c71fd8
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Thu Aug 8 17:22:57 2024 -0700
[util] modernize MemTracker
* use std::make_shared() to improve memory locality
* don't create unnecessary temporary objects
* remove static non-POD objects from the global scope
* switch to std::call_once() from GoogleOnceInit()
* other minor improvements
This patch doesn't contain any functional modifications.
Change-Id: I10e154055891cbec0a21a852629e8ed11ee61df1
Reviewed-on: http://gerrit.cloudera.org:8080/21654
Reviewed-by: Yingchun Lai <laiyingc...@apache.org>
Tested-by: Alexey Serbin <ale...@apache.org>
---
src/kudu/server/server_base.cc | 6 ++--
src/kudu/util/mem_tracker.cc | 69 ++++++++++++++++++------------------------
src/kudu/util/mem_tracker.h | 29 ++++++++++--------
3 files changed, 49 insertions(+), 55 deletions(-)
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index 2cc950a4a..1f770587b 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -606,7 +606,7 @@ shared_ptr<MemTracker> CreateMemTrackerForServer() {
if (id != 0) {
StrAppend(&id_str, " ", id);
}
- return shared_ptr<MemTracker>(MemTracker::CreateTracker(-1, id_str));
+ return MemTracker::CreateTracker(-1, id_str);
}
// Calculates the free space on the given WAL/data directory's disk. Returns
-1 in case of disk
@@ -736,8 +736,8 @@ ServerBase::ServerBase(string name, const
ServerBaseOptions& options,
GetFileCacheCapacity(options.env),
metric_entity_)),
rpc_server_(new RpcServer(options.rpc_opts)),
startup_path_handler_(new StartupPathHandler(metric_entity_)),
- result_tracker_(new rpc::ResultTracker(shared_ptr<MemTracker>(
- MemTracker::CreateTracker(-1, "result-tracker", mem_tracker_)))),
+ result_tracker_(new rpc::ResultTracker(
+ MemTracker::CreateTracker(-1, "result-tracker", mem_tracker_))),
is_first_run_(false),
stop_background_threads_latch_(1),
dns_resolver_(new DnsResolver(
diff --git a/src/kudu/util/mem_tracker.cc b/src/kudu/util/mem_tracker.cc
index fa4cc0d80..59b0518fe 100644
--- a/src/kudu/util/mem_tracker.cc
+++ b/src/kudu/util/mem_tracker.cc
@@ -18,27 +18,21 @@
#include "kudu/util/mem_tracker.h"
#include <algorithm>
-#include <cstddef>
#include <deque>
#include <limits>
#include <list>
#include <memory>
+#include <mutex>
#include <ostream>
#include <stack>
#include <type_traits>
-#include "kudu/gutil/once.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/util/mem_tracker.pb.h"
#include "kudu/util/mutex.h"
#include "kudu/util/process_memory.h"
-namespace kudu {
-
-// NOTE: this class has been adapted from Impala, so the code style varies
-// somewhat from kudu.
-
using std::deque;
using std::stack;
using std::list;
@@ -46,28 +40,18 @@ using std::shared_ptr;
using std::string;
using std::vector;
using std::weak_ptr;
-
using strings::Substitute;
-// The ancestor for all trackers. Every tracker is visible from the root down.
-static shared_ptr<MemTracker> root_tracker;
-static GoogleOnceType root_tracker_once = GOOGLE_ONCE_INIT;
+namespace kudu {
-void MemTracker::CreateRootTracker() {
- root_tracker.reset(new MemTracker(-1, "root", shared_ptr<MemTracker>()));
- root_tracker->Init();
-}
+// NOTE: this class has been adapted from Impala, so the code style varies
+// somewhat from kudu.
shared_ptr<MemTracker> MemTracker::CreateTracker(int64_t byte_limit,
const string& id,
shared_ptr<MemTracker>
parent) {
- shared_ptr<MemTracker> real_parent;
- if (parent) {
- real_parent = std::move(parent);
- } else {
- real_parent = GetRootTracker();
- }
- shared_ptr<MemTracker> tracker(new MemTracker(byte_limit, id, real_parent));
+ shared_ptr<MemTracker> real_parent = parent ? std::move(parent) :
GetRootTracker();
+ shared_ptr<MemTracker> tracker(MemTracker::make_shared(byte_limit, id,
real_parent));
real_parent->AddChildTracker(tracker);
tracker->Init();
@@ -120,7 +104,7 @@ bool MemTracker::FindTracker(const string& id,
bool MemTracker::FindTrackerInternal(const string& id,
shared_ptr<MemTracker>* tracker,
const shared_ptr<MemTracker>& parent) {
- DCHECK(parent != NULL);
+ DCHECK(parent);
list<weak_ptr<MemTracker>> children;
{
@@ -193,9 +177,9 @@ void
MemTracker::ListTrackers(vector<shared_ptr<MemTracker>>* trackers) {
}
void MemTracker::TrackersToPb(MemTrackerPB* pb) {
- CHECK(pb);
+ DCHECK(pb);
stack<std::pair<shared_ptr<MemTracker>, MemTrackerPB*>> to_process;
- to_process.emplace(std::make_pair(GetRootTracker(), pb));
+ to_process.emplace(GetRootTracker(), pb);
while (!to_process.empty()) {
auto tracker_and_pb = std::move(to_process.top());
to_process.pop();
@@ -214,13 +198,24 @@ void MemTracker::TrackersToPb(MemTrackerPB* pb) {
shared_ptr<MemTracker> child = child_weak.lock();
if (child) {
auto* child_pb = tracker_pb->add_child_trackers();
- to_process.emplace(std::make_pair(std::move(child), child_pb));
+ to_process.emplace(std::move(child), child_pb);
}
}
}
}
}
+shared_ptr<MemTracker> MemTracker::GetRootTracker() {
+ // The ancestor for all trackers. Every tracker is visible from the root
down.
+ static shared_ptr<MemTracker> root_tracker;
+ static std::once_flag once;
+ std::call_once(once, [&] {
+ root_tracker = MemTracker::make_shared(-1, "root",
std::shared_ptr<MemTracker>());
+ root_tracker->Init();
+ });
+ return root_tracker;
+}
+
void MemTracker::Consume(int64_t bytes) {
if (bytes < 0) {
Release(-bytes);
@@ -292,13 +287,12 @@ void MemTracker::Release(int64_t bytes) {
process_memory::MaybeGCAfterRelease(bytes);
}
-bool MemTracker::AnyLimitExceeded() {
- for (const auto& tracker : limit_trackers_) {
- if (tracker->LimitExceeded()) {
- return true;
- }
- }
- return false;
+bool MemTracker::AnyLimitExceeded() const {
+ return std::any_of(limit_trackers_.cbegin(),
+ limit_trackers_.cend(),
+ [](const auto& tracker) {
+ return tracker->LimitExceeded();
+ });
}
int64_t MemTracker::SpareCapacity() const {
@@ -316,7 +310,9 @@ void MemTracker::Init() {
MemTracker* tracker = this;
while (tracker) {
all_trackers_.push_back(tracker);
- if (tracker->has_limit()) limit_trackers_.push_back(tracker);
+ if (tracker->has_limit()) {
+ limit_trackers_.push_back(tracker);
+ }
tracker = tracker->parent_.get();
}
DCHECK_GT(all_trackers_.size(), 0);
@@ -328,9 +324,4 @@ void MemTracker::AddChildTracker(const
shared_ptr<MemTracker>& tracker) {
tracker->child_tracker_it_ = child_trackers_.insert(child_trackers_.end(),
tracker);
}
-shared_ptr<MemTracker> MemTracker::GetRootTracker() {
- GoogleOnceInit(&root_tracker_once, &MemTracker::CreateRootTracker);
- return root_tracker;
-}
-
} // namespace kudu
diff --git a/src/kudu/util/mem_tracker.h b/src/kudu/util/mem_tracker.h
index 867c528cf..e9a761861 100644
--- a/src/kudu/util/mem_tracker.h
+++ b/src/kudu/util/mem_tracker.h
@@ -27,6 +27,7 @@
#include <glog/logging.h>
#include "kudu/util/high_water_mark.h"
+#include "kudu/util/make_shared.h"
#include "kudu/util/mutex.h"
namespace kudu {
@@ -57,7 +58,8 @@ class MemTrackerPB;
// the tracker itself or to one of its descendants.
//
// This class is thread-safe.
-class MemTracker : public std::enable_shared_from_this<MemTracker> {
+class MemTracker : public std::enable_shared_from_this<MemTracker>,
+ public enable_make_shared<MemTracker> {
public:
~MemTracker();
@@ -71,7 +73,7 @@ class MemTracker : public
std::enable_shared_from_this<MemTracker> {
static std::shared_ptr<MemTracker> CreateTracker(
int64_t byte_limit,
const std::string& id,
- std::shared_ptr<MemTracker> parent = std::shared_ptr<MemTracker>());
+ std::shared_ptr<MemTracker> parent = {});
// If a tracker with the specified 'id' and 'parent' exists in the tree, sets
// 'tracker' to reference that instance. Returns false if no such tracker
@@ -84,7 +86,7 @@ class MemTracker : public
std::enable_shared_from_this<MemTracker> {
static bool FindTracker(
const std::string& id,
std::shared_ptr<MemTracker>* tracker,
- const std::shared_ptr<MemTracker>& parent =
std::shared_ptr<MemTracker>());
+ const std::shared_ptr<MemTracker>& parent = {});
// If a global tracker with the specified 'id' exists in the tree, returns a
// shared_ptr to that instance. Otherwise, creates a new MemTracker with the
@@ -125,12 +127,12 @@ class MemTracker : public
std::enable_shared_from_this<MemTracker> {
// Returns true if a valid limit of this tracker or one of its ancestors is
// exceeded.
- bool AnyLimitExceeded();
+ bool AnyLimitExceeded() const;
// If this tracker has a limit, checks the limit and attempts to free up
some memory if
// the limit is exceeded by calling any added GC functions. Returns true if
the limit is
// exceeded after calling the GC functions. Returns false if there is no
limit.
- bool LimitExceeded() {
+ bool LimitExceeded() const {
return limit_ >= 0 && limit_ < consumption();
}
@@ -158,17 +160,12 @@ class MemTracker : public
std::enable_shared_from_this<MemTracker> {
// guaranteed) to be globally unique.
std::string ToString() const;
- private:
+ protected:
// byte_limit < 0 means no limit
// 'id' is the label for LogUsage() and web UI.
MemTracker(int64_t byte_limit, const std::string& id,
std::shared_ptr<MemTracker> parent);
- // Further initializes the tracker.
- void Init();
-
- // Adds tracker to child_trackers_.
- void AddChildTracker(const std::shared_ptr<MemTracker>& tracker);
-
+ private:
// Variant of FindTracker() that must be called with a non-NULL parent.
static bool FindTrackerInternal(
const std::string& id,
@@ -178,7 +175,13 @@ class MemTracker : public
std::enable_shared_from_this<MemTracker> {
// Creates the root tracker.
static void CreateRootTracker();
- int64_t limit_;
+ // Further initializes the tracker.
+ void Init();
+
+ // Adds tracker to child_trackers_.
+ void AddChildTracker(const std::shared_ptr<MemTracker>& tracker);
+
+ const int64_t limit_;
const std::string id_;
const std::string descr_;
std::shared_ptr<MemTracker> parent_;
