Skip to content

Commit cf42bb1

Browse files
rkennkeclaude
andcommitted
fix(codecache): make memoryUsage() accurate and live
CodeCache::memoryUsage() drove the CODECACHE_NATIVE_SIZE_BYTES counter but both under-counted and went stale: - The blob array was counted as _capacity * sizeof(CodeBlob*) — a pointer's worth — while the array holds CodeBlob (three pointers), a ~3x undercount. - Symbol name strings were approximated as _count * sizeof(NativeFunc), a fixed 4 bytes each, ignoring the name length that dominates the actual allocation. - The DWARF unwind table and build-id string were not counted at all. - CodeCacheArray cached the sum at add() time, so it never reflected a library's later symbol growth. Recompute both accurately on demand instead. CodeCache::memoryUsage() now sums the full blob array, each symbol's real name-string allocation (via new NativeFunc::allocSize()), its own name, the DWARF table, and the build-id. CodeCacheArray::memoryUsage() sums the live per-library values (the array is append-only, so iterating the published prefix is safe alongside concurrent adds); the stale _used_memory cache is removed. memoryUsage() is only called at dump time, so the O(symbols) recompute is not on any hot path. Tests: new codeCache_ut asserts usage tracks per-symbol name length, that a longer name costs more than a shorter one, and that the blob array is counted at full CodeBlob size rather than a pointer's. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 77c2122 commit cf42bb1

3 files changed

Lines changed: 99 additions & 7 deletions

File tree

ddprof-lib/src/main/cpp/codeCache.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,28 @@ CodeCache::~CodeCache() {
144144
free(_build_id); // Free build-id memory
145145
}
146146

147+
long long CodeCache::memoryUsage() {
148+
// Blob array. The previous formula used sizeof(CodeBlob*), undercounting the
149+
// array roughly threefold (CodeBlob holds three pointers).
150+
long long total = (long long)_capacity * sizeof(CodeBlob);
151+
152+
// This cache's own name, plus each symbol's variable-length name string. The
153+
// previous formula approximated these as a fixed sizeof(NativeFunc), ignoring
154+
// the name length that dominates the allocation.
155+
total += (long long)NativeFunc::allocSize(_name);
156+
for (int i = 0; i < _count; i++) {
157+
total += (long long)NativeFunc::allocSize(_blobs[i]._name);
158+
}
159+
160+
// DWARF unwind table and build-id string were not counted at all before.
161+
total += (long long)_dwarf_table_length * sizeof(FrameDesc);
162+
if (_build_id != nullptr) {
163+
total += (long long)strlen(_build_id) + 1;
164+
}
165+
166+
return total;
167+
}
168+
147169
void CodeCache::expand() {
148170
CodeBlob *old_blobs = _blobs;
149171
CodeBlob *new_blobs = new CodeBlob[_capacity * 2];

ddprof-lib/src/main/cpp/codeCache.h

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,15 @@ class NativeFunc {
7474
static char *create(const char *name, short lib_index);
7575
static void destroy(char *name);
7676

77+
// Size of the heap allocation backing a name string produced by create().
78+
// Mirrors the size computed there so callers can account for it. 0 if null.
79+
static size_t allocSize(const char *name) {
80+
if (name == nullptr) {
81+
return 0;
82+
}
83+
return align_up(sizeof(NativeFunc) + 1 + strlen(name), sizeof(NativeFunc *));
84+
}
85+
7786
static short libIndex(const char *name) {
7887
if (name == nullptr) {
7988
return -1;
@@ -251,9 +260,11 @@ class CodeCache {
251260
void setDwarfTable(FrameDesc *table, int length, const FrameDesc &default_frame = FrameDesc::default_frame);
252261
FrameDesc findFrameDesc(const void *pc);
253262

254-
long long memoryUsage() {
255-
return _capacity * sizeof(CodeBlob *) + _count * sizeof(NativeFunc);
256-
}
263+
// Accurate live size of everything this CodeCache owns on the heap: the blob
264+
// array, the per-symbol name strings (variable length), the DWARF unwind
265+
// table, the build-id string, and this cache's own name. Recomputed on
266+
// demand (called only at dump time), so it always reflects current contents.
267+
long long memoryUsage();
257268

258269
int count() { return _count; }
259270
CodeBlob* blob(int idx) {
@@ -266,11 +277,10 @@ class CodeCacheArray {
266277
CodeCache *_libs[MAX_NATIVE_LIBS];
267278
volatile int _reserved; // next slot to reserve (CAS by writers)
268279
volatile int _count; // published count (all indices < _count have non-NULL pointers)
269-
volatile size_t _used_memory;
270280
bool _overflow_reported;
271281

272282
public:
273-
CodeCacheArray() : _reserved(0), _count(0), _used_memory(0), _overflow_reported(false) {
283+
CodeCacheArray() : _reserved(0), _count(0), _overflow_reported(false) {
274284
memset(_libs, 0, MAX_NATIVE_LIBS * sizeof(CodeCache *));
275285
}
276286

@@ -296,7 +306,6 @@ class CodeCacheArray {
296306
} while (!__atomic_compare_exchange_n(&_reserved, &slot, slot + 1,
297307
true, __ATOMIC_RELAXED, __ATOMIC_RELAXED));
298308
assert(__atomic_load_n(&_libs[slot], __ATOMIC_RELAXED) == nullptr);
299-
__atomic_fetch_add(&_used_memory, lib->memoryUsage(), __ATOMIC_RELAXED);
300309
// Store pointer before publishing count. The RELEASE here pairs with
301310
// the ACQUIRE load in operator[]/at() and count().
302311
__atomic_store_n(&_libs[slot], lib, __ATOMIC_RELEASE);
@@ -316,8 +325,20 @@ class CodeCacheArray {
316325
return __atomic_load_n(&_libs[index], __ATOMIC_ACQUIRE);
317326
}
318327

328+
// Sum the live memory of all registered libraries. Recomputed on demand
329+
// (called only at dump time) so it tracks each library's later growth,
330+
// unlike a value cached at add() time. The array is append-only, so
331+
// iterating the published prefix is safe alongside concurrent add()s.
319332
size_t memoryUsage() const {
320-
return __atomic_load_n(&_used_memory, __ATOMIC_RELAXED);
333+
size_t total = 0;
334+
int n = count();
335+
for (int i = 0; i < n; i++) {
336+
CodeCache *lib = at(i);
337+
if (lib != nullptr) {
338+
total += (size_t)lib->memoryUsage();
339+
}
340+
}
341+
return total;
321342
}
322343
};
323344

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright 2026, Datadog, Inc.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
#include <gtest/gtest.h>
7+
#include "codeCache.h"
8+
#include <cstring>
9+
10+
// memoryUsage() must reflect the actual per-symbol name length, not a fixed
11+
// per-symbol constant: adding one symbol grows the reported usage by exactly
12+
// the allocation size of its name string.
13+
TEST(CodeCacheTest, MemoryUsageCountsSymbolNameLength) {
14+
CodeCache cc("testlib");
15+
long long base = cc.memoryUsage();
16+
17+
const char *name =
18+
"a_long_symbol_name_well_beyond_sizeof_NativeFunc_padding_xxxxxxxxxxxx";
19+
cc.add((const void *)0x1000, 16, name);
20+
21+
EXPECT_EQ(NativeFunc::allocSize(name), (size_t)(cc.memoryUsage() - base));
22+
}
23+
24+
// A longer name costs more than a shorter one — i.e. length actually matters,
25+
// which the old fixed sizeof(NativeFunc) formula could not capture.
26+
TEST(CodeCacheTest, MemoryUsageGrowsWithNameLength) {
27+
CodeCache shortc("lib");
28+
CodeCache longc("lib");
29+
long long short_base = shortc.memoryUsage();
30+
long long long_base = longc.memoryUsage();
31+
32+
shortc.add((const void *)0x1000, 8, "f");
33+
longc.add((const void *)0x1000, 8,
34+
"an_extremely_long_symbol_name_padded_out_further_and_further_1234");
35+
36+
EXPECT_GT(longc.memoryUsage() - long_base, shortc.memoryUsage() - short_base);
37+
}
38+
39+
// The blob array is counted at the full CodeBlob size, not a pointer's worth
40+
// (the previous formula used sizeof(CodeBlob*), undercounting ~3x). An empty
41+
// cache already accounts for capacity * sizeof(CodeBlob) plus its own name.
42+
TEST(CodeCacheTest, MemoryUsageCountsFullBlobArray) {
43+
CodeCache cc("lib");
44+
long long usage = cc.memoryUsage();
45+
EXPECT_GT(usage, (long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)));
46+
// Sanity: a pointer-sized count would be far smaller than the real array.
47+
EXPECT_GT((long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)),
48+
(long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob *)));
49+
}

0 commit comments

Comments
 (0)