Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: allow slow and precise memory measurement of an object #4160

Merged
merged 1 commit into from
Nov 21, 2024
Merged
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
22 changes: 14 additions & 8 deletions src/core/compact_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,15 @@ constexpr XXH64_hash_t kHashSeed = 24061983;
constexpr size_t kAlignSize = 8u;

// Approximation since does not account for listpacks.
size_t QlMAllocSize(quicklist* ql) {
size_t res = ql->len * sizeof(quicklistNode) + znallocx(sizeof(quicklist));
return res + ql->count * 16; // we account for each member 16 bytes.
size_t QlMAllocSize(quicklist* ql, bool slow) {
size_t node_size = ql->len * sizeof(quicklistNode) + znallocx(sizeof(quicklist));
if (slow) {
for (quicklistNode* node = ql->head; node; node = node->next) {
node_size += zmalloc_usable_size(node->entry);
}
return node_size;
}
return node_size + ql->count * 16; // we account for each member 16 bytes.
}

inline void FreeObjSet(unsigned encoding, void* ptr, MemoryResource* mr) {
Expand Down Expand Up @@ -291,7 +297,7 @@ static_assert(sizeof(CompactObj) == 18);

namespace detail {

size_t RobjWrapper::MallocUsed() const {
size_t RobjWrapper::MallocUsed(bool slow) const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but isn't very elegant. Why don't you want to cache this size on the object like we do elsewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am planning to, once we switch to QList. Also with streams it may take some time as well.
Meanwhile I would like to have at least the "slow" variants because performance is not an issue for "memory usage".

if (!inner_obj_)
return 0;

Expand All @@ -301,8 +307,8 @@ size_t RobjWrapper::MallocUsed() const {
return InnerObjMallocUsed();
case OBJ_LIST:
if (encoding_ == OBJ_ENCODING_QUICKLIST)
return QlMAllocSize((quicklist*)inner_obj_);
return ((QList*)inner_obj_)->MallocUsed();
return QlMAllocSize((quicklist*)inner_obj_, slow);
return ((QList*)inner_obj_)->MallocUsed(slow);
case OBJ_SET:
return MallocUsedSet(encoding_, inner_obj_);
case OBJ_HASH:
Expand Down Expand Up @@ -1132,12 +1138,12 @@ void CompactObj::Free() {
memset(u_.inline_str, 0, kInlineLen);
}

size_t CompactObj::MallocUsed() const {
size_t CompactObj::MallocUsed(bool slow) const {
if (!HasAllocated())
return 0;

if (taglen_ == ROBJ_TAG) {
return u_.r_obj.MallocUsed();
return u_.r_obj.MallocUsed(slow);
}

if (taglen_ == JSON_TAG) {
Expand Down
8 changes: 4 additions & 4 deletions src/core/compact_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class RobjWrapper {
RobjWrapper() : sz_(0), type_(0), encoding_(0) {
}

size_t MallocUsed() const;
size_t MallocUsed(bool slow) const;

uint64_t HashCode() const;
bool Equal(const RobjWrapper& ow) const;
Expand Down Expand Up @@ -359,9 +359,9 @@ class CompactObj {
// Postcondition: The object is an in-memory string.
void Materialize(std::string_view str, bool is_raw);

// In case this object a single blob, returns number of bytes allocated on heap
// for that blob. Otherwise returns 0.
size_t MallocUsed() const;
// Returns the approximation of memory used by the object.
// If slow is true, may use more expensive methods to calculate the precise size.
size_t MallocUsed(bool slow = false) const;

// Resets the object to empty state (string).
void Reset();
Expand Down
13 changes: 10 additions & 3 deletions src/core/qlist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,17 @@ bool QList::Replace(long index, std::string_view elem) {
return false;
}

size_t QList::MallocUsed() const {
size_t QList::MallocUsed(bool slow) const {
// Approximation since does not account for listpacks.
size_t res = len_ * sizeof(quicklistNode) + znallocx(sizeof(quicklist));
return res + count_ * 16; // we account for each member 16 bytes.
size_t node_size = len_ * sizeof(quicklistNode) + znallocx(sizeof(quicklist));
if (slow) {
for (quicklistNode* node = head_; node; node = node->next) {
node_size += zmalloc_usable_size(node->entry);
}
return node_size;
}

return node_size + count_ * 16; // we account for each member 16 bytes.
}

void QList::Iterate(IterateFunc cb, long start, long end) const {
Expand Down
2 changes: 1 addition & 1 deletion src/core/qlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class QList {
// Returns true if item was replaced, false if index is out of range.
bool Replace(long index, std::string_view elem);

size_t MallocUsed() const;
size_t MallocUsed(bool slow) const;

void Iterate(IterateFunc cb, long start, long end) const;

Expand Down
4 changes: 4 additions & 0 deletions src/server/dfly_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -799,5 +799,9 @@ Usage: dragonfly [FLAGS]
unlink(pidfile_path.c_str());
}

// Returns memory to OS.
// This is a workaround for a bug in mi_malloc that may cause a crash on exit.
mi_collect(true);

return res;
}
15 changes: 15 additions & 0 deletions src/server/dragonfly_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,21 @@ TEST_F(DflyEngineTest, EvalBug2664) {
EXPECT_THAT(resp, DoubleArg(42.9));
}

TEST_F(DflyEngineTest, MemoryUsage) {
for (unsigned i = 0; i < 1000; ++i) {
Run({"rpush", "l1", StrCat("val", i)});
}

for (unsigned i = 0; i < 1000; ++i) {
Run({"rpush", "l2", StrCat(string('a', 200), i)});
}
auto resp = Run({"memory", "usage", "l1"});
EXPECT_GT(*resp.GetInt(), 8000);

resp = Run({"memory", "usage", "l2"});
EXPECT_GT(*resp.GetInt(), 100000);
}

// TODO: to test transactions with a single shard since then all transactions become local.
// To consider having a parameter in dragonfly engine controlling number of shards
// unconditionally from number of cpus. TO TEST BLPOP under multi for single/multi argument case.
Expand Down
3 changes: 2 additions & 1 deletion src/server/memory_cmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ std::string MallocStatsCb(bool backing, unsigned tid) {
}

size_t MemoryUsage(PrimeIterator it) {
return it->first.MallocUsed() + it->second.MallocUsed();
size_t key_size = it->first.MallocUsed();
return key_size + it->second.MallocUsed(true);
}

} // namespace
Expand Down
Loading