diff --git a/README.md b/README.md index a3799c34..675eddea 100644 --- a/README.md +++ b/README.md @@ -97,10 +97,14 @@ Install [`pprof`][npm-url] with `npm` or add to your `package.json`. pprof -http=: heap.pb.gz ``` - * Collecting a heap profile with V8 allocation profile format: + * Collecting a heap profile with V8 allocation profile format: ```javascript - const profile = await pprof.heap.v8Profile(); + const profile = pprof.heap.v8Profile(pprof.heap.convertProfile); ``` + `v8Profile` accepts a callback and returns its result. Allocation nodes + are only valid during the callback, so copy/transform what you need + before returning. `heap.convertProfile` performs that conversion during + the callback, and `heap.profile()` uses it under the hood. [build-image]: https://github.com/Datadog/pprof-nodejs/actions/workflows/build.yml/badge.svg?branch=main [build-url]: https://github.com/Datadog/pprof-nodejs/actions/workflows/build.yml diff --git a/bindings/profilers/heap.cc b/bindings/profilers/heap.cc index a5e3c435..ff7f2d01 100644 --- a/bindings/profilers/heap.cc +++ b/bindings/profilers/heap.cc @@ -488,23 +488,6 @@ NAN_METHOD(HeapProfiler::StopSamplingHeapProfiler) { } } -// Signature: -// getAllocationProfile(): AllocationProfileNode -NAN_METHOD(HeapProfiler::GetAllocationProfile) { - auto isolate = info.GetIsolate(); - std::unique_ptr profile( - isolate->GetHeapProfiler()->GetAllocationProfile()); - if (!profile) { - return Nan::ThrowError("Heap profiler is not enabled."); - } - v8::AllocationProfile::Node* root = profile->GetRootNode(); - auto state = PerIsolateData::For(isolate)->GetHeapProfilerState(); - if (state) { - state->OnNewProfile(); - } - info.GetReturnValue().Set(TranslateAllocationProfile(root)); -} - // mapAllocationProfile(callback): callback result NAN_METHOD(HeapProfiler::MapAllocationProfile) { if (info.Length() < 1 || !info[0]->IsFunction()) { @@ -596,7 +579,6 @@ NAN_MODULE_INIT(HeapProfiler::Init) { heapProfiler, "startSamplingHeapProfiler", StartSamplingHeapProfiler); Nan::SetMethod( heapProfiler, "stopSamplingHeapProfiler", StopSamplingHeapProfiler); - Nan::SetMethod(heapProfiler, "getAllocationProfile", GetAllocationProfile); Nan::SetMethod(heapProfiler, "mapAllocationProfile", MapAllocationProfile); Nan::SetMethod(heapProfiler, "monitorOutOfMemory", MonitorOutOfMemory); Nan::Set(target, diff --git a/bindings/profilers/heap.hh b/bindings/profilers/heap.hh index aef0ef7e..d87e1cac 100644 --- a/bindings/profilers/heap.hh +++ b/bindings/profilers/heap.hh @@ -30,10 +30,6 @@ class HeapProfiler { // stopSamplingHeapProfiler() static NAN_METHOD(StopSamplingHeapProfiler); - // Signature: - // getAllocationProfile(): AllocationProfileNode - static NAN_METHOD(GetAllocationProfile); - // Signature: // mapAllocationProfile(callback): callback result static NAN_METHOD(MapAllocationProfile); diff --git a/bindings/translate-heap-profile.cc b/bindings/translate-heap-profile.cc index 41f5e129..0e795dd3 100644 --- a/bindings/translate-heap-profile.cc +++ b/bindings/translate-heap-profile.cc @@ -41,30 +41,6 @@ class HeapProfileTranslator : ProfileTranslator { #undef X public: - v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node) { - v8::Local children = NewArray(node->children.size()); - for (size_t i = 0; i < node->children.size(); i++) { - Set(children, i, TranslateAllocationProfile(node->children[i])); - } - - v8::Local allocations = NewArray(node->allocations.size()); - for (size_t i = 0; i < node->allocations.size(); i++) { - auto alloc = node->allocations[i]; - Set(allocations, - i, - CreateAllocation(NewNumber(alloc.count), NewNumber(alloc.size))); - } - - return CreateNode(node->name, - node->script_name, - NewInteger(node->script_id), - NewInteger(node->line_number), - NewInteger(node->column_number), - children, - allocations); - } - v8::Local TranslateAllocationProfile(Node* node) { v8::Local children = NewArray(node->children.size()); for (size_t i = 0; i < node->children.size(); i++) { @@ -142,11 +118,6 @@ std::shared_ptr TranslateAllocationProfileToCpp( return new_node; } -v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node) { - return HeapProfileTranslator().TranslateAllocationProfile(node); -} - v8::Local TranslateAllocationProfile(Node* node) { return HeapProfileTranslator().TranslateAllocationProfile(node); } diff --git a/bindings/translate-heap-profile.hh b/bindings/translate-heap-profile.hh index e913dd66..22b644b7 100644 --- a/bindings/translate-heap-profile.hh +++ b/bindings/translate-heap-profile.hh @@ -39,7 +39,4 @@ std::shared_ptr TranslateAllocationProfileToCpp( v8::AllocationProfile::Node* node); v8::Local TranslateAllocationProfile(Node* node); -v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node); - } // namespace dd diff --git a/ts/src/heap-profiler-bindings.ts b/ts/src/heap-profiler-bindings.ts index f5baa8e8..e6fa0b05 100644 --- a/ts/src/heap-profiler-bindings.ts +++ b/ts/src/heap-profiler-bindings.ts @@ -37,10 +37,6 @@ export function stopSamplingHeapProfiler() { profiler.heapProfiler.stopSamplingHeapProfiler(); } -export function getAllocationProfile(): AllocationProfileNode { - return profiler.heapProfiler.getAllocationProfile(); -} - export function mapAllocationProfile( callback: (root: AllocationProfileNode) => T ): T { diff --git a/ts/src/heap-profiler.ts b/ts/src/heap-profiler.ts index afe08e75..550121e6 100644 --- a/ts/src/heap-profiler.ts +++ b/ts/src/heap-profiler.ts @@ -17,7 +17,6 @@ import {Profile} from 'pprof-format'; import { - getAllocationProfile, mapAllocationProfile, startSamplingHeapProfiler, stopSamplingHeapProfiler, @@ -34,20 +33,6 @@ import {isMainThread} from 'worker_threads'; let enabled = false; let heapIntervalBytes = 0; let heapStackDepth = 0; - -/* - * Collects a heap profile when heapProfiler is enabled. Otherwise throws - * an error. - * - * Data is returned in V8 allocation profile format. - */ -export function v8Profile(): AllocationProfileNode { - if (!enabled) { - throw new Error('Heap profiler is not enabled.'); - } - return getAllocationProfile(); -} - /** * Collects a heap profile when heapProfiler is enabled. Otherwise throws * an error. @@ -59,35 +44,13 @@ export function v8Profile(): AllocationProfileNode { * @param callback - function to convert the heap profiler to a converted profile * @returns converted profile */ -export function v8ProfileV2( - callback: (root: AllocationProfileNode) => T -): T { +export function v8Profile(callback: (root: AllocationProfileNode) => T): T { if (!enabled) { throw new Error('Heap profiler is not enabled.'); } return mapAllocationProfile(callback); } -/** - * Collects a profile and returns it serialized in pprof format. - * Throws if heap profiler is not enabled. - * - * @param ignoreSamplePath - * @param sourceMapper - */ -export function profile( - ignoreSamplePath?: string, - sourceMapper?: SourceMapper, - generateLabels?: GenerateAllocationLabelsFunction -): Profile { - return convertProfile( - v8Profile(), - ignoreSamplePath, - sourceMapper, - generateLabels - ); -} - export function convertProfile( rootNode: AllocationProfileNode, ignoreSamplePath?: string, @@ -130,12 +93,12 @@ export function convertProfile( * @param sourceMapper * @param generateLabels */ -export function profileV2( +export function profile( ignoreSamplePath?: string, sourceMapper?: SourceMapper, generateLabels?: GenerateAllocationLabelsFunction ): Profile { - return v8ProfileV2(root => { + return v8Profile(root => { return convertProfile(root, ignoreSamplePath, sourceMapper, generateLabels); }); } diff --git a/ts/src/index.ts b/ts/src/index.ts index be2a4170..42454629 100644 --- a/ts/src/index.ts +++ b/ts/src/index.ts @@ -48,7 +48,6 @@ export const heap = { start: heapProfiler.start, stop: heapProfiler.stop, profile: heapProfiler.profile, - profileV2: heapProfiler.profileV2, convertProfile: heapProfiler.convertProfile, v8Profile: heapProfiler.v8Profile, monitorOutOfMemory: heapProfiler.monitorOutOfMemory, diff --git a/ts/test/heap-memory-worker.ts b/ts/test/heap-memory-worker.ts deleted file mode 100644 index fa7038c5..00000000 --- a/ts/test/heap-memory-worker.ts +++ /dev/null @@ -1,112 +0,0 @@ -import * as heapProfiler from '../src/heap-profiler'; -import * as v8HeapProfiler from '../src/heap-profiler-bindings'; -import {AllocationProfileNode} from '../src/v8-types'; - -const gc = (global as NodeJS.Global & {gc?: () => void}).gc; -if (!gc) { - throw new Error('Run with --expose-gc flag'); -} - -const keepAlive: object[] = []; - -// Create many unique functions via new Function() to produce a large profile tree. -function createAllocatorFunctions(count: number): Function[] { - const fns: Function[] = []; - for (let i = 0; i < count; i++) { - const fn = new Function( - 'keepAlive', - ` - for (let j = 0; j < 100; j++) { - keepAlive.push({ - id${i}: j, - data${i}: new Array(64).fill('${'x'.repeat(16)}'), - }); - } - ` - ); - fns.push(() => fn(keepAlive)); - } - return fns; -} - -function createDeepChain(depth: number): Function[] { - const fns: Function[] = []; - for (let i = depth - 1; i >= 0; i--) { - const next = i < depth - 1 ? fns[fns.length - 1] : null; - const fn = new Function( - 'keepAlive', - 'next', - ` - for (let j = 0; j < 50; j++) { - keepAlive.push({ arr${i}: new Array(32).fill(j) }); - } - if (next) next(keepAlive, null); - ` - ) as (arr: object[], next: unknown) => void; - fns.push((arr: object[]) => fn(arr, next)); - } - return fns; -} - -function generateAllocations(): void { - const wideFns = createAllocatorFunctions(5000); - for (const fn of wideFns) { - fn(); - } - - for (let chain = 0; chain < 200; chain++) { - const deepFns = createDeepChain(50); - deepFns[deepFns.length - 1](keepAlive); - } -} - -function traverseTree(root: AllocationProfileNode): void { - const stack: AllocationProfileNode[] = [root]; - while (stack.length > 0) { - const node = stack.pop()!; - if (node.children) { - for (const child of node.children) { - stack.push(child); - } - } - } -} - -function measureV1(): {initial: number; afterTraversal: number} { - gc!(); - gc!(); - const baseline = process.memoryUsage().heapUsed; - - const profile = v8HeapProfiler.getAllocationProfile(); - const initial = process.memoryUsage().heapUsed - baseline; - traverseTree(profile); - const afterTraversal = process.memoryUsage().heapUsed - baseline; - - return {initial, afterTraversal}; -} - -function measureV2(): {initial: number; afterTraversal: number} { - gc!(); - gc!(); - const baseline = process.memoryUsage().heapUsed; - - return v8HeapProfiler.mapAllocationProfile(root => { - const initial = process.memoryUsage().heapUsed - baseline; - traverseTree(root); - const afterTraversal = process.memoryUsage().heapUsed - baseline; - return {initial, afterTraversal}; - }); -} - -process.on('message', (version: 'v1' | 'v2') => { - heapProfiler.start(128, 128); - generateAllocations(); - - const {initial, afterTraversal} = - version === 'v1' ? measureV1() : measureV2(); - - heapProfiler.stop(); - keepAlive.length = 0; - - process.send!({initial, afterTraversal}); -}); diff --git a/ts/test/test-heap-profiler-v2.ts b/ts/test/test-heap-profiler-v2.ts deleted file mode 100644 index d514c241..00000000 --- a/ts/test/test-heap-profiler-v2.ts +++ /dev/null @@ -1,130 +0,0 @@ -import {strict as assert} from 'assert'; -import {fork} from 'child_process'; -import * as heapProfiler from '../src/heap-profiler'; -import * as v8HeapProfiler from '../src/heap-profiler-bindings'; - -function generateAllocations(): object[] { - const allocations: object[] = []; - for (let i = 0; i < 1000; i++) { - allocations.push({data: new Array(100).fill(i)}); - } - return allocations; -} - -describe('HeapProfiler V2 API', () => { - let keepAlive: object[] = []; - - before(() => { - heapProfiler.start(512, 64); - keepAlive = generateAllocations(); - }); - - after(() => { - heapProfiler.stop(); - keepAlive.length = 0; - }); - - describe('v8ProfileV2', () => { - it('should return AllocationProfileNode', () => { - heapProfiler.v8ProfileV2(root => { - assert.equal(typeof root.name, 'string'); - assert.equal(typeof root.scriptName, 'string'); - assert.equal(typeof root.scriptId, 'number'); - assert.equal(typeof root.lineNumber, 'number'); - assert.equal(typeof root.columnNumber, 'number'); - assert.ok(Array.isArray(root.allocations)); - - assert.ok(Array.isArray(root.children)); - assert.equal(typeof root.children.length, 'number'); - - if (root.children.length > 0) { - const child = root.children[0]; - assert.equal(typeof child.name, 'string'); - assert.ok(Array.isArray(child.children)); - assert.ok(Array.isArray(child.allocations)); - } - }); - }); - - it('should throw error when profiler not started', () => { - heapProfiler.stop(); - assert.throws( - () => { - heapProfiler.v8ProfileV2(() => {}); - }, - (err: Error) => { - return err.message === 'Heap profiler is not enabled.'; - } - ); - heapProfiler.start(512, 64); - }); - }); - - describe('mapAllocationProfile', () => { - it('should return AllocationProfileNode directly', () => { - v8HeapProfiler.mapAllocationProfile(root => { - assert.equal(typeof root.name, 'string'); - assert.equal(typeof root.scriptName, 'string'); - assert.ok(Array.isArray(root.children)); - assert.ok(Array.isArray(root.allocations)); - }); - }); - }); - - describe('profileV2', () => { - it('should produce valid pprof Profile', () => { - const profile = heapProfiler.profileV2(); - - assert.ok(profile.sampleType); - assert.ok(profile.sample); - assert.ok(profile.location); - assert.ok(profile.function); - assert.ok(profile.stringTable); - }); - }); - - describe('Memory comparison', () => { - interface MemoryResult { - initial: number; - afterTraversal: number; - } - - function measureMemoryInWorker( - version: 'v1' | 'v2' - ): Promise { - return new Promise((resolve, reject) => { - const child = fork('./out/test/heap-memory-worker.js', [], { - execArgv: ['--expose-gc'], - }); - - child.on('message', (result: MemoryResult) => { - resolve(result); - child.kill(); - }); - - child.on('error', reject); - child.send(version); - }); - } - - it('mapAllocationProfile should use less initial memory than getAllocationProfile', async () => { - const v1MemoryUsage = await measureMemoryInWorker('v1'); - const v2MemoryUsage = await measureMemoryInWorker('v2'); - - console.log( - ` V1 initial: ${v1MemoryUsage.initial}, afterTraversal: ${v1MemoryUsage.afterTraversal} - | V2 initial: ${v2MemoryUsage.initial}, afterTraversal: ${v2MemoryUsage.afterTraversal}` - ); - - assert.ok( - v2MemoryUsage.initial < v1MemoryUsage.initial, - `V2 initial should be less: V1=${v1MemoryUsage.initial}, V2=${v2MemoryUsage.initial}` - ); - - assert.ok( - v2MemoryUsage.afterTraversal < v1MemoryUsage.afterTraversal, - `V2 afterTraversal should be less: V1=${v1MemoryUsage.afterTraversal}, V2=${v2MemoryUsage.afterTraversal}` - ); - }).timeout(100_000); - }); -}); diff --git a/ts/test/test-heap-profiler.ts b/ts/test/test-heap-profiler.ts index 178ca283..6ffe259d 100644 --- a/ts/test/test-heap-profiler.ts +++ b/ts/test/test-heap-profiler.ts @@ -35,10 +35,30 @@ import { const copy = require('deep-copy'); const assert = require('assert'); +function mapToGetterNode(node: AllocationProfileNode): AllocationProfileNode { + const children = (node.children || []).map(mapToGetterNode); + const allocations = node.allocations || []; + const result = {}; + + Object.defineProperties(result, { + name: {get: () => node.name}, + scriptName: {get: () => node.scriptName}, + scriptId: {get: () => node.scriptId}, + lineNumber: {get: () => node.lineNumber}, + columnNumber: {get: () => node.columnNumber}, + allocations: {get: () => allocations}, + children: {get: () => children}, + }); + return result as AllocationProfileNode; +} + describe('HeapProfiler', () => { let startStub: sinon.SinonStub<[number, number], void>; let stopStub: sinon.SinonStub<[], void>; - let profileStub: sinon.SinonStub<[], AllocationProfileNode>; + let profileStub: sinon.SinonStub< + [(root: AllocationProfileNode) => unknown], + unknown + >; let dateStub: sinon.SinonStub<[], number>; let memoryUsageStub: sinon.SinonStub<[], NodeJS.MemoryUsage>; beforeEach(() => { @@ -58,8 +78,8 @@ describe('HeapProfiler', () => { describe('profile', () => { it('should return a profile equal to the expected profile when external memory is allocated', async () => { profileStub = sinon - .stub(v8HeapProfiler, 'getAllocationProfile') - .returns(copy(v8HeapProfile)); + .stub(v8HeapProfiler, 'mapAllocationProfile') + .callsFake(callback => callback(mapToGetterNode(copy(v8HeapProfile)))); memoryUsageStub = sinon.stub(process, 'memoryUsage').returns({ external: 1024, rss: 2048, @@ -76,8 +96,10 @@ describe('HeapProfiler', () => { it('should return a profile equal to the expected profile when including all samples', async () => { profileStub = sinon - .stub(v8HeapProfiler, 'getAllocationProfile') - .returns(copy(v8HeapWithPathProfile)); + .stub(v8HeapProfiler, 'mapAllocationProfile') + .callsFake(callback => + callback(mapToGetterNode(copy(v8HeapWithPathProfile))) + ); memoryUsageStub = sinon.stub(process, 'memoryUsage').returns({ external: 0, rss: 2048, @@ -94,8 +116,10 @@ describe('HeapProfiler', () => { it('should return a profile equal to the expected profile when excluding profiler samples', async () => { profileStub = sinon - .stub(v8HeapProfiler, 'getAllocationProfile') - .returns(copy(v8HeapWithPathProfile)); + .stub(v8HeapProfiler, 'mapAllocationProfile') + .callsFake(callback => + callback(mapToGetterNode(copy(v8HeapWithPathProfile))) + ); memoryUsageStub = sinon.stub(process, 'memoryUsage').returns({ external: 0, rss: 2048, @@ -112,8 +136,10 @@ describe('HeapProfiler', () => { it('should return a profile equal to the expected profile when adding labels', async () => { profileStub = sinon - .stub(v8HeapProfiler, 'getAllocationProfile') - .returns(copy(v8HeapWithPathProfile)); + .stub(v8HeapProfiler, 'mapAllocationProfile') + .callsFake(callback => + callback(mapToGetterNode(copy(v8HeapWithPathProfile))) + ); memoryUsageStub = sinon.stub(process, 'memoryUsage').returns({ external: 0, rss: 2048,