From 7373665eb7819e91086c4a09daeb8b60111752f8 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Tue, 4 Jun 2024 16:39:18 +0200 Subject: [PATCH 01/29] feat(pprof): initial commit --- pnpm-lock.yaml | 6 ++++++ provider/pprof/README.md | 24 ++++++++++++++++++++++++ provider/pprof/index.test.ts | 13 +++++++++++++ provider/pprof/index.ts | 29 +++++++++++++++++++++++++++++ provider/pprof/package.json | 28 ++++++++++++++++++++++++++++ provider/pprof/tsconfig.json | 11 +++++++++++ provider/pprof/vitest.config.ts | 3 +++ 7 files changed, 114 insertions(+) create mode 100644 provider/pprof/README.md create mode 100644 provider/pprof/index.test.ts create mode 100644 provider/pprof/index.ts create mode 100644 provider/pprof/package.json create mode 100644 provider/pprof/tsconfig.json create mode 100644 provider/pprof/vitest.config.ts diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7a874611..38eaa584 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -658,6 +658,12 @@ importers: specifier: workspace:* version: link:../../lib/provider + provider/pprof: + dependencies: + '@openctx/provider': + specifier: workspace:* + version: link:../../lib/provider + provider/prometheus: dependencies: '@openctx/provider': diff --git a/provider/pprof/README.md b/provider/pprof/README.md new file mode 100644 index 00000000..c8989254 --- /dev/null +++ b/provider/pprof/README.md @@ -0,0 +1,24 @@ +# [pprof](https://github.com/google/pprof) context provider for OpenCtx + +This is a context provider for [OpenCtx](https://openctx.org) that annotates functions with the CPU time and memory allocations attributed to them. + +## Usage + +Add the following to your settings in any OpenCtx client: + +```json +"openctx.providers": { + // ...other providers... + "https://openctx.org/npm/@openctx/provider-pprof": true +}, +``` + +## Configuration + +> TODO + +## Development + +- [Source code](https://sourcegraph.com/github.com/sourcegraph/openctx/-/tree/provider/pprof) +- [Docs](https://openctx.org/docs/providers/pprof) +- License: Apache 2.0 diff --git a/provider/pprof/index.test.ts b/provider/pprof/index.test.ts new file mode 100644 index 00000000..6e9ee561 --- /dev/null +++ b/provider/pprof/index.test.ts @@ -0,0 +1,13 @@ +import type { MetaResult } from '@openctx/provider' +import { describe, expect, test } from 'vitest' +import pprof from './index.js' + +describe('pprof', () => { + test('meta', () => + expect(pprof.meta({}, {})).toStrictEqual({ + name: 'pprof', + annotations: { + selectors: [{ path: '**/*.go' }], + }, + })) +}) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts new file mode 100644 index 00000000..c99e3482 --- /dev/null +++ b/provider/pprof/index.ts @@ -0,0 +1,29 @@ +import type { + AnnotationsParams, + AnnotationsResult, + MetaParams, + MetaResult, + Provider, + ProviderSettings, +} from '@openctx/provider' + +/** + * An [OpenCtx](https://openctx.org) provider that annotates every function declaration with + * the CPU time and memory allocations associated with it. + */ +const pprof: Provider = { + meta(params: MetaParams, settings: ProviderSettings): MetaResult { + return { + name: 'pprof', + annotations: { + selectors: [{ path: '**/*.go' }], + }, + } + }, + + annotations(params: AnnotationsParams, settings: ProviderSettings): AnnotationsResult { + return [] + }, +} + +export default pprof diff --git a/provider/pprof/package.json b/provider/pprof/package.json new file mode 100644 index 00000000..363ce0ef --- /dev/null +++ b/provider/pprof/package.json @@ -0,0 +1,28 @@ +{ + "name": "@openctx/provider-pprof", + "version": "0.0.13", + "description": "pprof (OpenCtx provider)", + "license": "Apache-2.0", + "homepage": "https://openctx.org/docs/providers/pprof", + "repository": { + "type": "git", + "url": "https://github.com/sourcegraph/openctx", + "directory": "provider/pprof" + }, + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "files": [ + "dist/index.js", + "dist/index.d.ts" + ], + "sideEffects": false, + "scripts": { + "build": "tsc --build", + "prepublishOnly": "tsc --build --clean && pnpm run --silent build", + "test": "vitest" + }, + "dependencies": { + "@openctx/provider": "workspace:*" + } +} diff --git a/provider/pprof/tsconfig.json b/provider/pprof/tsconfig.json new file mode 100644 index 00000000..4ff7ab81 --- /dev/null +++ b/provider/pprof/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "../../.config/tsconfig.base.json", + "compilerOptions": { + "rootDir": ".", + "outDir": "dist", + "lib": ["ESNext"], + }, + "include": ["*.ts"], + "exclude": ["dist", "vitest.config.ts"], + "references": [{ "path": "../../lib/provider" }], +} diff --git a/provider/pprof/vitest.config.ts b/provider/pprof/vitest.config.ts new file mode 100644 index 00000000..abed6b21 --- /dev/null +++ b/provider/pprof/vitest.config.ts @@ -0,0 +1,3 @@ +import { defineConfig } from 'vitest/config' + +export default defineConfig({}) From 6d5a52cdf581c21b02de3da7025ca904b25308e6 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 5 Jun 2024 00:23:10 +0200 Subject: [PATCH 02/29] feat(pprof): calculate Range for each func/method --- provider/pprof/parser.test.ts | 57 +++++++++++++++++++++ provider/pprof/parser.ts | 94 +++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 provider/pprof/parser.test.ts create mode 100644 provider/pprof/parser.ts diff --git a/provider/pprof/parser.test.ts b/provider/pprof/parser.test.ts new file mode 100644 index 00000000..08b892ea --- /dev/null +++ b/provider/pprof/parser.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, test } from 'vitest' +import parseGolang, { type Func } from './parser.js' + +describe('file parsing', () => { + test('golang', () => { + const content = `package example + +import "fmt" + +func A(a string) {} +func b_func() {} +func A2() { + fmt.Print("hello pprof") +} + +type Thing struct {} + +func (t *Thing) doStuff(i int) {} +func (t Thing) String() string { return "thing" } + ` + + expect(parseGolang(content)).toStrictEqual([ + { + package: 'example', + function: 'A', + range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } }, + pprofRegex: 'example.A', + }, + { + package: 'example', + function: 'b_func', + range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } }, + pprofRegex: 'example.b_func', + }, + { + package: 'example', + function: 'A2', + range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } }, + pprofRegex: 'example.A2', + }, + { + package: 'example', + function: 'doStuff', + range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } }, + pprofRegex: 'example.(*Thing).doStuff', + receiver: '*Thing', + }, + { + package: 'example', + function: 'String', + range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } }, + pprofRegex: 'example.(Thing).String', + receiver: 'Thing', + }, + ]) + }) +}) diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts new file mode 100644 index 00000000..7e517c06 --- /dev/null +++ b/provider/pprof/parser.ts @@ -0,0 +1,94 @@ +import type { Range } from '@openctx/provider' + +const packageRegex = /^package (\w+)/m +const funcRegex = /^func (\w+)(?:\()/m +const methodRegex = /^func \(\w+ (\*)?(\w+)\) (\w+)(?:\()/m + +export interface Func { + package: string + function: string + range: Range + pprofRegex: string + receiver?: string +} + +export default function parseGolang(source: string): Func[] { + const result: Func[] = [] + + const pkgMatch = packageRegex.exec(source) + if (!pkgMatch || !pkgMatch.length) { + return result + } + const pkg = pkgMatch[1] + + const lines = source.split('\n') + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + + const readFuncName = (start: number): { func: string; end: number } => { + let end = start + for (const ch of line.substring(start).split('')) { + if (ch === '(') { + break + } + end++ + } + return { + func: line.substring(start, end), + end: end, + } + } + + switch (true) { + case funcRegex.test(line): { + const start = 5 // "func ".length + const { func, end } = readFuncName(start) + + result.push({ + package: pkg, + function: func, + range: { + start: { line: i, character: start }, + end: { line: i, character: end }, + }, + pprofRegex: `${pkg}.${func}`, + }) + break + } + case methodRegex.test(line): { + const lparen = 5 // "func ".length + let rparen = lparen + + for (const ch of line.substring(lparen).split('')) { + if (ch === ')') { + break + } + rparen++ + } + + let receiver = line.substring(lparen, rparen) + receiver = receiver.split(' ')[1] + if (!receiver) { + continue + } + + const start = rparen + 2 + const { func, end } = readFuncName(start) + + result.push({ + package: pkg, + function: func, + range: { + start: { line: i, character: start }, + end: { line: i, character: end }, + }, + pprofRegex: `${pkg}.(${receiver}).${func}`, + receiver: receiver, + }) + break + } + } + } + + return result +} From 0240819cca53917b23fa8c065743c32fcae11053 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 5 Jun 2024 00:36:24 +0200 Subject: [PATCH 03/29] fix(pprof): escape special characters for pprof search --- provider/pprof/parser.test.ts | 10 +++++----- provider/pprof/parser.ts | 13 +++++++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/provider/pprof/parser.test.ts b/provider/pprof/parser.test.ts index 08b892ea..813029a5 100644 --- a/provider/pprof/parser.test.ts +++ b/provider/pprof/parser.test.ts @@ -24,32 +24,32 @@ func (t Thing) String() string { return "thing" } package: 'example', function: 'A', range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } }, - pprofRegex: 'example.A', + pprofRegex: 'example\\.A', }, { package: 'example', function: 'b_func', range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } }, - pprofRegex: 'example.b_func', + pprofRegex: 'example\\.b_func', }, { package: 'example', function: 'A2', range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } }, - pprofRegex: 'example.A2', + pprofRegex: 'example\\.A2', }, { package: 'example', function: 'doStuff', range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } }, - pprofRegex: 'example.(*Thing).doStuff', + pprofRegex: 'example\\.\\(\\*Thing\\)\\.doStuff', receiver: '*Thing', }, { package: 'example', function: 'String', range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } }, - pprofRegex: 'example.(Thing).String', + pprofRegex: 'example\\.\\(Thing\\)\\.String', receiver: 'Thing', }, ]) diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts index 7e517c06..36b04820 100644 --- a/provider/pprof/parser.ts +++ b/provider/pprof/parser.ts @@ -51,7 +51,7 @@ export default function parseGolang(source: string): Func[] { start: { line: i, character: start }, end: { line: i, character: end }, }, - pprofRegex: `${pkg}.${func}`, + pprofRegex: escapeSpecial(`${pkg}.${func}`), }) break } @@ -82,7 +82,7 @@ export default function parseGolang(source: string): Func[] { start: { line: i, character: start }, end: { line: i, character: end }, }, - pprofRegex: `${pkg}.(${receiver}).${func}`, + pprofRegex: escapeSpecial(`${pkg}.(${receiver}).${func}`), receiver: receiver, }) break @@ -92,3 +92,12 @@ export default function parseGolang(source: string): Func[] { return result } + +/** + * Escape all special regex characters in a string. + * @param s string + * @returns string + */ +function escapeSpecial(s: string): string { + return s.replace(/[().*]/g, '\\$&') +} From 0a01fbdd2fe8f241c4b6a2006d9c7a89f465fb12 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 5 Jun 2024 15:44:58 +0200 Subject: [PATCH 04/29] wip(pprof): search pprof report and go binary Before generating annotations for a file, we want to check if 1) go is installed (to use `go tool pprof`) 2) a pprof report that matches the configured glob exists in one of the parent directories --- provider/pprof/pprof.test.ts | 95 ++++++++++++++++++++++++++++++++++++ provider/pprof/pprof.ts | 74 ++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 provider/pprof/pprof.test.ts create mode 100644 provider/pprof/pprof.ts diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts new file mode 100644 index 00000000..77fc38f3 --- /dev/null +++ b/provider/pprof/pprof.test.ts @@ -0,0 +1,95 @@ +import { readdirSync } from 'fs' +import { type ExecException, exec } from 'node:child_process' +import { beforeEach, describe, expect, test, vi } from 'vitest' +import { findReportPath, getPprof } from './pprof.js' + +vi.mock('node:child_process', () => ({ exec: vi.fn() })) +const execMock = vi.mocked(exec) + +vi.mock('fs', () => ({ readdirSync: vi.fn() })) +const readdirSyncMock = vi.mocked(readdirSync) + +describe('pprof', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + test('get pprof (installed)', () => { + execMock.mockImplementationOnce(mockExec('/usr/local/go/bin') as unknown as typeof exec) + + const pprof = getPprof() + + expect(exec).toHaveBeenCalledOnce() + expect(pprof).not.toBeNull() + }) + + test('get pprof (not installed)', () => { + execMock.mockImplementationOnce(mockExec('go not found') as unknown as typeof exec) + + const pprof = getPprof() + + expect(exec).toHaveBeenCalledOnce() + expect(pprof).toBeNull() + }) + + test('find report (exists)', () => { + readdirSyncMock.mockImplementation(((s: string): string[] => { + return s === '/path/to' ? ['report.pprof'] : [] + }) as unknown as typeof readdirSync) + + const profilePath = findReportPath('/path/to/current', { + reportGlob: '*.pprof', + workspaceRoot: '/path', + }) + + expect(profilePath).toBe('/path/to/report.pprof') + }) + + test('find report with rootDirectoryMarkers (does not exist)', () => { + readdirSyncMock.mockImplementation(((s: string): string[] => { + switch (s) { + case '/': + // Should not reach this case + return ['other.pprof'] + case '/root': + // Contains root directory markers + return ['README.md', '.git'] + default: + return [] + } + }) as unknown as typeof readdirSync) + + const profilePath = findReportPath('/root/to/current', { + reportGlob: '*.pprof', + rootDirectoryMarkers: ['.git'], + }) + + expect(profilePath).toBeNull() + }) + + test('find report with workspaceRoot (does not exist)', () => { + readdirSyncMock.mockImplementation(((s: string): string[] => { + return [] + }) as unknown as typeof readdirSync) + + const profilePath = findReportPath('/path/to/current', { + reportGlob: '*.pprof', + workspaceRoot: '/path', + }) + + expect(profilePath).toBeNull() + }) + + test.todo('top') + test.todo('list') +}) + +type ExecCallback = (error: ExecException | null, stdout: string, stderr: string) => void + +function mockExec(stdout: string): (command: string, callback?: ExecCallback) => void { + return (_command: string, callback?: ExecCallback) => { + if (callback) { + callback(null, stdout, '') + } + } +} diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts new file mode 100644 index 00000000..95e3f979 --- /dev/null +++ b/provider/pprof/pprof.ts @@ -0,0 +1,74 @@ +import { exec } from 'child_process' +import { readdirSync } from 'fs' +import { dirname, join } from 'path' +import { matchGlob } from '@openctx/provider' + +export function getPprof(): Pprof | null { + let hasGo = false + exec('which go', (error, stdout, stderr) => { + if (error !== null || stdout.endsWith('not found')) { + return + } + hasGo = true + }) + + if (!hasGo) { + return null + } + return new Pprof() +} + +interface SearchOptions { + reportGlob: string + workspaceRoot?: string + rootDirectoryMarkers?: string[] + binaryGlob?: string // TODO: find binary if possible +} + +export function findReportPath(currentDir: string, options: SearchOptions): string | null { + const matchReport = matchGlob(options.reportGlob) + // const matchBinary = options.binaryGlob ? matchGlob(options.binaryGlob) : (s: string) => falses + + const reachedRoot = (dir: string): boolean => { + if (options.workspaceRoot !== undefined) { + return options.workspaceRoot === dir + } + if (!options.rootDirectoryMarkers?.length) { + return false + } + + const markers = options.rootDirectoryMarkers + try { + return readdirSync(dir).some(f => markers.includes(f)) + } catch { + return false + } + } + + let report: string | null = null + let searchDir = currentDir + + Search: while (true) { + let contents: string[] = [] + try { + contents = readdirSync(searchDir) + } catch { + return null + } + + for (const file of contents) { + if (matchReport(file)) { + report = join(searchDir, file) + break Search + } + } + + if (reachedRoot(searchDir) || searchDir === '/') { + return null + } + searchDir = dirname(searchDir) + } + return report +} + +export class Pprof {} From 9465d797b90493555f4f82aac1b7f4d1a9bbd602 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 6 Jun 2024 14:00:29 +0200 Subject: [PATCH 05/29] feat(pprof): get `pprof -top` for current package This makes sure the annotations are only set for the most "resource hungry" functions and not for each function in the package. --- provider/pprof/pprof.test.ts | 88 ++++++++++++++++++- provider/pprof/pprof.ts | 160 ++++++++++++++++++++++++++++++++++- 2 files changed, 242 insertions(+), 6 deletions(-) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 77fc38f3..9a504acd 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -1,7 +1,14 @@ import { readdirSync } from 'fs' import { type ExecException, exec } from 'node:child_process' import { beforeEach, describe, expect, test, vi } from 'vitest' -import { findReportPath, getPprof } from './pprof.js' +import { + Pprof, + type PprofTool, + type TopOptions, + type TopOutput, + findReportPath, + getPprof, +} from './pprof.js' vi.mock('node:child_process', () => ({ exec: vi.fn() })) const execMock = vi.mocked(exec) @@ -80,14 +87,89 @@ describe('pprof', () => { expect(profilePath).toBeNull() }) - test.todo('top') + type TopCmdTest = { name: string; tool: PprofTool; opts: TopOptions; binary?: string; want: string } + + test.each([ + { + name: 'defaults', + tool: 'go tool pprof', + opts: { package: 'main' }, + want: `go tool pprof -top -show="main\\." -cum -noinlines report.pprof`, + }, + { + name: 'defaults', + tool: 'go tool pprof', + opts: { package: 'main' }, + binary: './my-binary', + want: `go tool pprof -top -show="main\\." -cum -noinlines ./my-binary report.pprof`, + }, + { + name: 'sort flat', + tool: 'go tool pprof', + opts: { package: 'main', sort: 'flat' }, + binary: './my-binary', + want: `go tool pprof -top -show="main\\." -flat -noinlines ./my-binary report.pprof`, + }, + { + name: 'include inline', + tool: 'go tool pprof', + opts: { package: 'main', excludeInline: false }, + want: `go tool pprof -top -show="main\\." -cum report.pprof`, + }, + ])('top command ($name)', (tt: TopCmdTest) => { + execMock.mockImplementationOnce(mockExec('') as unknown as typeof exec) + const pprof = new Pprof(tt.tool) + pprof.setReport('report.pprof') + pprof.setBinary(tt.binary) + + pprof.top(tt.opts) + + expect(execMock).toHaveBeenCalledOnce() + expect(exec).toHaveBeenCalledWith(tt.want, expect.any(Function)) + }) + + test.only('top (CPU)', () => { + const outputCpu = `File: pprof-example +Type: cpu +Time: Jun 1, 2024 at 10:56pm (CEST) +Duration: 8.39s, Total samples = 13.02s (155.11%) +Active filters: + show=^main\. +Showing nodes accounting for 6.25s, 48.00% of 13.02s total +Dropped 2 nodes (cum <= 0.07s) +Showing top 3 nodes out of 7 + flat flat% sum% cum cum% + 0.03s 0.23% 0.23% 6.38s 49.00% main.main + 5.59s 42.93% 43.16% 6.21s 47.70% main.Run + 0.63s 4.84% 48.00% 0.63s 4.84% pkg/list.(*L).Init` + + const tool: PprofTool = 'go tool pprof' + const topOptions: TopOptions = { package: 'main' } + execMock.mockImplementationOnce(mockExec(outputCpu) as unknown as typeof exec) + + const pprof = new Pprof(tool) + pprof.setReport('/path/to/report.pprof') + + const top = pprof.top(topOptions) + + expect(top).toStrictEqual({ + file: 'pprof-example', + type: 'cpu', + unit: 's', + nodes: [ + { function: 'main.main', flat: 0.03, flatPerc: 0.23, cum: 6.38, cumPerc: 49 }, + { function: 'main.Run', flat: 5.59, flatPerc: 42.93, cum: 6.21, cumPerc: 47.7 }, + { function: 'pkg/list.(*L).Init', flat: 0.63, flatPerc: 4.84, cum: 0.63, cumPerc: 4.84 }, + ], + }) + }) test.todo('list') }) type ExecCallback = (error: ExecException | null, stdout: string, stderr: string) => void function mockExec(stdout: string): (command: string, callback?: ExecCallback) => void { - return (_command: string, callback?: ExecCallback) => { + return (command: string, callback?: ExecCallback) => { if (callback) { callback(null, stdout, '') } diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 95e3f979..03943157 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -3,9 +3,30 @@ import { readdirSync } from 'fs' import { dirname, join } from 'path' import { matchGlob } from '@openctx/provider' +/** + * topNodeRegex is the output in `pprof -top` command for a single node + * + * An example output would look like this (first row for reference only): + * + * ``` + * flat flat% sum% cum cum% + * 5.59s 42.93% 43.16% 6.21s 47.70% container/list.(*List).Init + * ``` + * + * @see https://regex101.com/r/26v9rd/1 + */ +const topNodeRegex = + /^ +(?[\d\\.]+)(?\w) +(?[\d\\.]+)% +(?:[\d\\.]+)% +(?[\d\\.]+)(?:\w) +(?[\d\\.]+)% +(?[\w\\.\\(\\*\\)\\/]+)/ + +/** + * PprofTool is a CLI tool for reading and visualizing profile reports. + * `pprof` is bundled with every `go` binary, but may also be installed as a standalone tool. + */ +export type PprofTool = 'go tool pprof' | 'pprof' + export function getPprof(): Pprof | null { let hasGo = false - exec('which go', (error, stdout, stderr) => { + exec('which go', (error, stdout, _stderr) => { if (error !== null || stdout.endsWith('not found')) { return } @@ -15,7 +36,7 @@ export function getPprof(): Pprof | null { if (!hasGo) { return null } - return new Pprof() + return new Pprof('go tool pprof') } interface SearchOptions { @@ -71,4 +92,137 @@ export function findReportPath(currentDir: string, options: SearchOptions): stri return report } -export class Pprof {} +export interface TopOptions { + /** package limits the entries to the current package (`-show="^package\."`)*/ + package: string + + /** nodeCount limits the number of results returned (`-nodecount=x`). Skipped if not set. */ + nodeCount?: number + + /** excludeInline excludes inline function calls from the output (`-noinlines`) */ + excludeInline?: boolean + + /** sort controls how nodes are sorted in the output (`-flat` or `-cum`) */ + sort?: 'flat' | 'cum' +} + +export interface TopOutput { + /** Name of the binary used to generate the report. If no binary was passed to `pprof` command, this field is `undefined`. */ + file?: string + type: string + unit: string + nodes: Node[] +} + +export interface Node { + function: string + + /** Absolute CPU time/ memory associated with this function only. */ + flat: number + + /** Relative CPU time / memory associated with this function only. */ + flatPerc: number + + /** CPU time / memory associated with the function and its descendants. */ + cum: number + + /** Relative CPU time / memory associated with the function and its descendants. */ + cumPerc: number +} + +/** + * Pprof is a wrapper for working with `pprof` CLI. + */ +export class Pprof { + private tool: PprofTool + private report: string | null = null + private binary?: string + + constructor(tool: PprofTool) { + this.tool = tool + } + + setReport(path: string) { + this.report = path + } + + setBinary(path?: string) { + this.binary = path + } + + top(options: TopOptions): TopOutput | null { + if (!this.report) { + return null + } + + const cmd = this.topCmd(options) + let out: string | null = null + exec(cmd, (error, stdout, stderr) => { + if (error !== null || stderr.endsWith('no such file or directory')) { + return + } + out = stdout + }) + + if (out === null) { + return null + } + + return this.parseTop(out) + } + + private topCmd(options: TopOptions): string { + let cmd = this.tool + ` -top -show="${options.package}\\."` + cmd += options.sort ? ` -${options.sort}` : ' -cum' + + if (options.excludeInline === undefined || options.excludeInline === true) { + cmd += ' -noinlines' + } + + if (this.binary) { + cmd += ` ${this.binary}` + } + cmd += ` ${this.report}` + return cmd + } + + private parseTop(output: string): TopOutput | null { + const binaryName = /^File: ([\w-\\/]+)/m.exec(output) + const reportType = /^Type: (\w+)/m.exec(output) + + let unit: string | null = null + const nodes: Node[] = [] + + const startPos = output.search(/ +flat +flat% +sum% +cum +cum%/) + if (startPos === -1) { + return null + } + const lines = output.substring(startPos).split('\n').slice(1) + for (const line of lines) { + const match = topNodeRegex.exec(line) + if (match === null || !match.groups) { + continue + } + const { groups: node } = match + + if (unit === null) { + unit = node.unit + } + + nodes.push({ + function: node.func, + flat: parseFloat(node.flat), + flatPerc: parseFloat(node.flatPerc), + cum: parseFloat(node.cum), + cumPerc: parseFloat(node.cumPerc), + }) + } + + return { + type: reportType ? reportType[1] || 'cpu' : 'cpu', + file: binaryName ? binaryName[1] : undefined, + unit: unit || 's', + nodes: nodes, + } + } +} From 2692b18f0f769487d815f9b15265ce4300920b11 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 6 Jun 2024 14:02:22 +0200 Subject: [PATCH 06/29] fix(pprof/parser): espace all special characters in the string --- provider/pprof/parser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts index 36b04820..d156ee5d 100644 --- a/provider/pprof/parser.ts +++ b/provider/pprof/parser.ts @@ -99,5 +99,5 @@ export default function parseGolang(source: string): Func[] { * @returns string */ function escapeSpecial(s: string): string { - return s.replace(/[().*]/g, '\\$&') + return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&') } From b36759b580903cf2f5cdac5993a580fbbd6f55b5 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 6 Jun 2024 18:38:39 +0200 Subject: [PATCH 07/29] feat(pprof): annotate Go files Putting it all together! `profider-pprof` can now annotate Go source files with information about the CPU time the functions/methods defined in it take. This requires `go` to be installed (standalone `pprof` will be supported too) and a report generated. Some remarks: - Bungling to --format=cjs until this fix https://github.com/sourcegraph/openctx/pull/131 is merged - _log.ts file is a temporary fixture for local development - Lots of TODOs to handle --- provider/pprof/_log.ts | 35 ++++++++++++++++++ provider/pprof/index.ts | 62 +++++++++++++++++++++++++++++++- provider/pprof/package.json | 9 ++--- provider/pprof/parser.test.ts | 68 +++++++++++++++++------------------ provider/pprof/parser.ts | 31 +++++++++------- provider/pprof/pprof.test.ts | 34 +++++++++--------- provider/pprof/pprof.ts | 35 ++++++++++-------- 7 files changed, 189 insertions(+), 85 deletions(-) create mode 100644 provider/pprof/_log.ts diff --git a/provider/pprof/_log.ts b/provider/pprof/_log.ts new file mode 100644 index 00000000..66b5712d --- /dev/null +++ b/provider/pprof/_log.ts @@ -0,0 +1,35 @@ +/** + * _log is a local debugging util that writes logs to a log file at $HOME/log/openctx/provider-pprof.log. + * I could not find a way to log things to VSCode Output, so I came up with this workaround. + * + * This file is not imported anywhere, so no directories will be created on your machine + * with `pprof` provider activated enabled. + * + * It's only a temporary fixture -- there's probably a better solution to this. + */ + +import { appendFileSync, closeSync, mkdirSync, openSync, statSync } from 'fs' +import { join } from 'path' + +const logDir = `${process.env.HOME}/log/openctx` +const logFile = join(logDir, 'provider-pprof.log') + +try { + statSync(logDir) +} catch { + mkdirSync(logDir, { recursive: true }) +} +closeSync(openSync(logFile, 'w')) + +/** + * DEBUG writes logs to $HOME/log/openctx/provider-pprof.log + * To watch the logs run: + * + * ``` + * tail -f $HOME/log/openctx/provider-pprof.log + * ``` + */ +export default function DEBUG(message?: any, ...args: any[]): void { + const now = new Date(Date.now()).toUTCString() + appendFileSync(logFile, `[${now}] ${message}${args.join(' ')}` + '\n') +} diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index c99e3482..61021db8 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -1,4 +1,6 @@ +import { dirname } from 'path' import type { + Annotation, AnnotationsParams, AnnotationsResult, MetaParams, @@ -6,6 +8,8 @@ import type { Provider, ProviderSettings, } from '@openctx/provider' +import { escapeSpecial, parseGolang } from './parser.js' +import { type Node, findReportPath, getPprof } from './pprof.js' /** * An [OpenCtx](https://openctx.org) provider that annotates every function declaration with @@ -22,7 +26,63 @@ const pprof: Provider = { }, annotations(params: AnnotationsParams, settings: ProviderSettings): AnnotationsResult { - return [] + // TODO: check it is not a test file, skip otherwise + + const pprof = getPprof() + if (pprof === null) { + // TODO: log that no command line tool was found. Ideally, do it once on init. + return [] + } + + // let workspaceRoot: string | undefined = undefined + // const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.parse(params.uri)) + // if (workspaceFolder && workspaceFolder !== null) { + // workspaceRoot = workspaceFolder.uri.path + // } + + const searchDir = dirname(params.uri).replace(/^file:\/{2}/, '') + const report = findReportPath(searchDir, { + reportGlob: (settings.reportGlob as string) || '**/*pb.gz', + // workspaceRoot: workspaceRoot, + rootDirectoryMarkers: settings.rootDirectoryMarkers as string[], + }) + if (report === null) { + return [] + } + pprof.setReport(report) + + const content = parseGolang(params.content) + if (!content) { + return [] + } + + const top = pprof.top({ package: content.package }) + if (top === null) { + return [] + } + + const anns: Annotation[] = [] + + // TODO(1): turn Func[] into a Record for faster lookups. + // TODO(2): do not escape pprofRegex, delay it until it's used in `pprof -list`. + // This way we do not need to do the awkward conversion of `node.function` to match it. + for (const func of content.funcs) { + top.nodes.forEach((node: Node, i: number) => { + if (func.pprofRegex !== escapeSpecial(node.function)) { + return + } + + anns.push({ + uri: params.uri, + range: func.range, + item: { + title: `[#${i + 1}] CPU Time: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`, + }, + }) + }) + } + + return anns }, } diff --git a/provider/pprof/package.json b/provider/pprof/package.json index 363ce0ef..5a476b15 100644 --- a/provider/pprof/package.json +++ b/provider/pprof/package.json @@ -12,17 +12,18 @@ "type": "module", "main": "dist/index.js", "types": "dist/index.d.ts", - "files": [ - "dist/index.js", - "dist/index.d.ts" - ], + "files": ["dist/index.js", "dist/index.d.ts"], "sideEffects": false, "scripts": { "build": "tsc --build", + "bundle": "tsc --build && esbuild --log-level=error --bundle --format=cjs --platform=node --external:vscode --outfile=dist/bundle.js index.ts", "prepublishOnly": "tsc --build --clean && pnpm run --silent build", "test": "vitest" }, "dependencies": { "@openctx/provider": "workspace:*" + }, + "devDependencies": { + "@types/vscode": "^1.85.0" } } diff --git a/provider/pprof/parser.test.ts b/provider/pprof/parser.test.ts index 813029a5..3dd28194 100644 --- a/provider/pprof/parser.test.ts +++ b/provider/pprof/parser.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest' -import parseGolang, { type Func } from './parser.js' +import { type Contents, parseGolang } from './parser.js' describe('file parsing', () => { test('golang', () => { @@ -19,39 +19,37 @@ func (t *Thing) doStuff(i int) {} func (t Thing) String() string { return "thing" } ` - expect(parseGolang(content)).toStrictEqual([ - { - package: 'example', - function: 'A', - range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } }, - pprofRegex: 'example\\.A', - }, - { - package: 'example', - function: 'b_func', - range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } }, - pprofRegex: 'example\\.b_func', - }, - { - package: 'example', - function: 'A2', - range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } }, - pprofRegex: 'example\\.A2', - }, - { - package: 'example', - function: 'doStuff', - range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } }, - pprofRegex: 'example\\.\\(\\*Thing\\)\\.doStuff', - receiver: '*Thing', - }, - { - package: 'example', - function: 'String', - range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } }, - pprofRegex: 'example\\.\\(Thing\\)\\.String', - receiver: 'Thing', - }, - ]) + expect(parseGolang(content)).toStrictEqual({ + package: 'example', + funcs: [ + { + name: 'A', + range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } }, + pprofRegex: 'example\\.A', + }, + { + name: 'b_func', + range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } }, + pprofRegex: 'example\\.b_func', + }, + { + name: 'A2', + range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } }, + pprofRegex: 'example\\.A2', + }, + { + name: 'doStuff', + range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } }, + pprofRegex: 'example\\.\\(\\*Thing\\)\\.doStuff', + receiver: '*Thing', + }, + { + name: 'String', + range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } }, + pprofRegex: 'example\\.\\(Thing\\)\\.String', + receiver: 'Thing', + }, + ], + }) }) }) diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts index d156ee5d..10f42fa1 100644 --- a/provider/pprof/parser.ts +++ b/provider/pprof/parser.ts @@ -4,22 +4,29 @@ const packageRegex = /^package (\w+)/m const funcRegex = /^func (\w+)(?:\()/m const methodRegex = /^func \(\w+ (\*)?(\w+)\) (\w+)(?:\()/m -export interface Func { +export interface Contents { package: string - function: string + funcs: Func[] +} + +/** Func is a Go function or method with additional metadata to help locate it in the file and filter for in in `pprof`. */ +export interface Func { + name: string range: Range pprofRegex: string receiver?: string } -export default function parseGolang(source: string): Func[] { - const result: Func[] = [] - +export function parseGolang(source: string): Contents | null { const pkgMatch = packageRegex.exec(source) if (!pkgMatch || !pkgMatch.length) { - return result + return null } const pkg = pkgMatch[1] + const result: Contents = { + package: pkg, + funcs: [], + } const lines = source.split('\n') for (let i = 0; i < lines.length; i++) { @@ -44,9 +51,8 @@ export default function parseGolang(source: string): Func[] { const start = 5 // "func ".length const { func, end } = readFuncName(start) - result.push({ - package: pkg, - function: func, + result.funcs.push({ + name: func, range: { start: { line: i, character: start }, end: { line: i, character: end }, @@ -75,9 +81,8 @@ export default function parseGolang(source: string): Func[] { const start = rparen + 2 const { func, end } = readFuncName(start) - result.push({ - package: pkg, - function: func, + result.funcs.push({ + name: func, range: { start: { line: i, character: start }, end: { line: i, character: end }, @@ -98,6 +103,6 @@ export default function parseGolang(source: string): Func[] { * @param s string * @returns string */ -function escapeSpecial(s: string): string { +export function escapeSpecial(s: string): string { return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&') } diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 9a504acd..8127b954 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -1,5 +1,5 @@ +import { execSync } from 'child_process' import { readdirSync } from 'fs' -import { type ExecException, exec } from 'node:child_process' import { beforeEach, describe, expect, test, vi } from 'vitest' import { Pprof, @@ -10,8 +10,8 @@ import { getPprof, } from './pprof.js' -vi.mock('node:child_process', () => ({ exec: vi.fn() })) -const execMock = vi.mocked(exec) +vi.mock('child_process', () => ({ execSync: vi.fn() })) +const execSyncMock = vi.mocked(execSync) vi.mock('fs', () => ({ readdirSync: vi.fn() })) const readdirSyncMock = vi.mocked(readdirSync) @@ -22,20 +22,22 @@ describe('pprof', () => { }) test('get pprof (installed)', () => { - execMock.mockImplementationOnce(mockExec('/usr/local/go/bin') as unknown as typeof exec) + execSyncMock.mockImplementationOnce( + mockExecSync('/usr/local/go/bin') as unknown as typeof execSync + ) const pprof = getPprof() - expect(exec).toHaveBeenCalledOnce() + expect(execSync).toHaveBeenCalledOnce() expect(pprof).not.toBeNull() }) test('get pprof (not installed)', () => { - execMock.mockImplementationOnce(mockExec('go not found') as unknown as typeof exec) + execSyncMock.mockImplementationOnce(mockExecSync('go not found') as unknown as typeof execSync) const pprof = getPprof() - expect(exec).toHaveBeenCalledOnce() + expect(execSync).toHaveBeenCalledOnce() expect(pprof).toBeNull() }) @@ -117,15 +119,15 @@ describe('pprof', () => { want: `go tool pprof -top -show="main\\." -cum report.pprof`, }, ])('top command ($name)', (tt: TopCmdTest) => { - execMock.mockImplementationOnce(mockExec('') as unknown as typeof exec) + execSyncMock.mockImplementationOnce(mockExecSync('') as unknown as typeof execSync) const pprof = new Pprof(tt.tool) pprof.setReport('report.pprof') pprof.setBinary(tt.binary) pprof.top(tt.opts) - expect(execMock).toHaveBeenCalledOnce() - expect(exec).toHaveBeenCalledWith(tt.want, expect.any(Function)) + expect(execSyncMock).toHaveBeenCalledOnce() + expect(execSync).toHaveBeenCalledWith(tt.want, expect.any(Function)) }) test.only('top (CPU)', () => { @@ -145,7 +147,7 @@ Showing top 3 nodes out of 7 const tool: PprofTool = 'go tool pprof' const topOptions: TopOptions = { package: 'main' } - execMock.mockImplementationOnce(mockExec(outputCpu) as unknown as typeof exec) + execSyncMock.mockImplementationOnce(mockExecSync(outputCpu) as unknown as typeof execSync) const pprof = new Pprof(tool) pprof.setReport('/path/to/report.pprof') @@ -166,12 +168,8 @@ Showing top 3 nodes out of 7 test.todo('list') }) -type ExecCallback = (error: ExecException | null, stdout: string, stderr: string) => void - -function mockExec(stdout: string): (command: string, callback?: ExecCallback) => void { - return (command: string, callback?: ExecCallback) => { - if (callback) { - callback(null, stdout, '') - } +function mockExecSync(stdout: string): (command: string) => Buffer { + return (_command: string) => { + return Buffer.from(stdout, 'utf-8') } } diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 03943157..be40d804 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -1,4 +1,4 @@ -import { exec } from 'child_process' +import { execSync } from 'child_process' import { readdirSync } from 'fs' import { dirname, join } from 'path' import { matchGlob } from '@openctx/provider' @@ -26,12 +26,13 @@ export type PprofTool = 'go tool pprof' | 'pprof' export function getPprof(): Pprof | null { let hasGo = false - exec('which go', (error, stdout, _stderr) => { - if (error !== null || stdout.endsWith('not found')) { - return - } + + try { + execSync('which go') hasGo = true - }) + } catch (e) { + return null + } if (!hasGo) { return null @@ -73,11 +74,15 @@ export function findReportPath(currentDir: string, options: SearchOptions): stri let contents: string[] = [] try { contents = readdirSync(searchDir) - } catch { + } catch (e) { return null } for (const file of contents) { + // TODO: search for binary + // Note, that by breaking the loop after finding the report we assume that the binary + // is located in in the same directories or in one of the directories we've searched before. + // Which is a rather fair assumption. if (matchReport(file)) { report = join(searchDir, file) break Search @@ -89,6 +94,7 @@ export function findReportPath(currentDir: string, options: SearchOptions): stri } searchDir = dirname(searchDir) } + return report } @@ -150,21 +156,22 @@ export class Pprof { this.binary = path } + // TODO: return raw output top(options: TopOptions): TopOutput | null { if (!this.report) { return null } const cmd = this.topCmd(options) + let out: string | null = null - exec(cmd, (error, stdout, stderr) => { - if (error !== null || stderr.endsWith('no such file or directory')) { - return - } - out = stdout - }) + try { + out = execSync(cmd).toString('utf-8').trim() + } catch (e) { + return null + } - if (out === null) { + if (out === null || out.includes('no such file or directory')) { return null } From 83e687fd3c1739c45dcf9d3d0c031fd1798f0234 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Fri, 7 Jun 2024 11:13:44 +0200 Subject: [PATCH 08/29] test(pprof): enable all tests and simplify mocking execSync does not return an error if the command fails, so we check that its output is not "not found". --- provider/pprof/pprof.test.ts | 26 ++++++++++---------------- provider/pprof/pprof.ts | 4 +++- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 8127b954..31f663b5 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -22,9 +22,7 @@ describe('pprof', () => { }) test('get pprof (installed)', () => { - execSyncMock.mockImplementationOnce( - mockExecSync('/usr/local/go/bin') as unknown as typeof execSync - ) + execSyncMock.mockReturnValueOnce(buffer('/usr/local/go/bin')) const pprof = getPprof() @@ -33,7 +31,7 @@ describe('pprof', () => { }) test('get pprof (not installed)', () => { - execSyncMock.mockImplementationOnce(mockExecSync('go not found') as unknown as typeof execSync) + execSyncMock.mockReturnValueOnce(buffer('go not found')) const pprof = getPprof() @@ -77,9 +75,7 @@ describe('pprof', () => { }) test('find report with workspaceRoot (does not exist)', () => { - readdirSyncMock.mockImplementation(((s: string): string[] => { - return [] - }) as unknown as typeof readdirSync) + readdirSyncMock.mockReturnValueOnce([]) const profilePath = findReportPath('/path/to/current', { reportGlob: '*.pprof', @@ -99,7 +95,7 @@ describe('pprof', () => { want: `go tool pprof -top -show="main\\." -cum -noinlines report.pprof`, }, { - name: 'defaults', + name: 'include binary', tool: 'go tool pprof', opts: { package: 'main' }, binary: './my-binary', @@ -119,7 +115,7 @@ describe('pprof', () => { want: `go tool pprof -top -show="main\\." -cum report.pprof`, }, ])('top command ($name)', (tt: TopCmdTest) => { - execSyncMock.mockImplementationOnce(mockExecSync('') as unknown as typeof execSync) + execSyncMock.mockReturnValueOnce(buffer('')) const pprof = new Pprof(tt.tool) pprof.setReport('report.pprof') pprof.setBinary(tt.binary) @@ -127,10 +123,10 @@ describe('pprof', () => { pprof.top(tt.opts) expect(execSyncMock).toHaveBeenCalledOnce() - expect(execSync).toHaveBeenCalledWith(tt.want, expect.any(Function)) + expect(execSync).toHaveBeenCalledWith(tt.want) }) - test.only('top (CPU)', () => { + test('top (CPU)', () => { const outputCpu = `File: pprof-example Type: cpu Time: Jun 1, 2024 at 10:56pm (CEST) @@ -147,7 +143,7 @@ Showing top 3 nodes out of 7 const tool: PprofTool = 'go tool pprof' const topOptions: TopOptions = { package: 'main' } - execSyncMock.mockImplementationOnce(mockExecSync(outputCpu) as unknown as typeof execSync) + execSyncMock.mockReturnValueOnce(buffer(outputCpu)) const pprof = new Pprof(tool) pprof.setReport('/path/to/report.pprof') @@ -168,8 +164,6 @@ Showing top 3 nodes out of 7 test.todo('list') }) -function mockExecSync(stdout: string): (command: string) => Buffer { - return (_command: string) => { - return Buffer.from(stdout, 'utf-8') - } +function buffer(s: string): Buffer { + return Buffer.from(s, 'utf-8') } diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index be40d804..544afa87 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -28,8 +28,10 @@ export function getPprof(): Pprof | null { let hasGo = false try { - execSync('which go') + const stdout = execSync('which go').toString('utf-8').trim() + if (!stdout.endsWith('not found')) { hasGo = true + } } catch (e) { return null } From 33dbd71441977778e8f1e95126471b920823f07c Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Fri, 7 Jun 2024 11:17:25 +0200 Subject: [PATCH 09/29] fix(pprof): missing dot in default reportGlob --- provider/pprof/_log.ts | 2 +- provider/pprof/index.ts | 2 +- provider/pprof/pprof.ts | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/provider/pprof/_log.ts b/provider/pprof/_log.ts index 66b5712d..3c2a928a 100644 --- a/provider/pprof/_log.ts +++ b/provider/pprof/_log.ts @@ -3,7 +3,7 @@ * I could not find a way to log things to VSCode Output, so I came up with this workaround. * * This file is not imported anywhere, so no directories will be created on your machine - * with `pprof` provider activated enabled. + * with `pprof` provider enabled. * * It's only a temporary fixture -- there's probably a better solution to this. */ diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index 61021db8..e422489e 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -42,7 +42,7 @@ const pprof: Provider = { const searchDir = dirname(params.uri).replace(/^file:\/{2}/, '') const report = findReportPath(searchDir, { - reportGlob: (settings.reportGlob as string) || '**/*pb.gz', + reportGlob: (settings.reportGlob as string) || '**/*.pb.gz', // workspaceRoot: workspaceRoot, rootDirectoryMarkers: settings.rootDirectoryMarkers as string[], }) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 544afa87..b7425d22 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -30,7 +30,7 @@ export function getPprof(): Pprof | null { try { const stdout = execSync('which go').toString('utf-8').trim() if (!stdout.endsWith('not found')) { - hasGo = true + hasGo = true } } catch (e) { return null @@ -202,6 +202,7 @@ export class Pprof { let unit: string | null = null const nodes: Node[] = [] + // Find the table with per-function stats and discard the headers const startPos = output.search(/ +flat +flat% +sum% +cum +cum%/) if (startPos === -1) { return null From 194c62de7927fea0876218601cb020c912005b67 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Fri, 7 Jun 2024 11:49:54 +0200 Subject: [PATCH 10/29] chore(pprof): return no annotations for a test file --- provider/pprof/index.test.ts | 38 +++++++++++++++++++++++++++++++++++- provider/pprof/index.ts | 5 ++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/provider/pprof/index.test.ts b/provider/pprof/index.test.ts index 6e9ee561..f5d209ee 100644 --- a/provider/pprof/index.test.ts +++ b/provider/pprof/index.test.ts @@ -1,8 +1,29 @@ +import { beforeEach } from 'node:test' import type { MetaResult } from '@openctx/provider' -import { describe, expect, test } from 'vitest' +import { beforeAll, describe, expect, test, vi } from 'vitest' import pprof from './index.js' +import { getPprof } from './pprof.js' + +vi.mock('./pprof.js', async () => { + const actualPprof = await vi.importActual('./pprof.js') + return { ...actualPprof, getPprof: vi.fn() } +}) +const getPprofMock = vi.mocked(getPprof) describe('pprof', () => { + let actualPprof: typeof import('./pprof.js') | undefined + beforeAll(async () => { + actualPprof = await vi.importActual('./pprof.js') + }) + + beforeEach(async () => { + vi.clearAllMocks() + + // All tests should use the actual implementation, we just want to spy on the calls. + // We also know that the original module is available, because it's imported in the beforeAll hook. + getPprofMock.mockImplementationOnce(actualPprof!.getPprof) + }) + test('meta', () => expect(pprof.meta({}, {})).toStrictEqual({ name: 'pprof', @@ -10,4 +31,19 @@ describe('pprof', () => { selectors: [{ path: '**/*.go' }], }, })) + + test('annotations for a test file', () => { + const content = 'package pkg_test\nfunc DoStuff() {}\n' + + expect( + pprof.annotations!( + { + uri: '/pkg/thing_test.go', + content: content, + }, + {} + ) + ).toHaveLength(0) + expect(getPprofMock).not.toBeCalled() + }) }) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index e422489e..cf9b6efd 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -26,7 +26,10 @@ const pprof: Provider = { }, annotations(params: AnnotationsParams, settings: ProviderSettings): AnnotationsResult { - // TODO: check it is not a test file, skip otherwise + // Test files do not need pprof annotations. + if (params.uri.endsWith('_test.go')) { + return [] + } const pprof = getPprof() if (pprof === null) { From 8f6d41d81f04f7866b23e01c3a00a89fd8639b74 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Fri, 7 Jun 2024 11:55:35 +0200 Subject: [PATCH 11/29] chore(pprof): drop 'vscode' dependency, remove unused code --- provider/pprof/index.ts | 9 ++------- provider/pprof/package.json | 5 +---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index cf9b6efd..f942faca 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -37,17 +37,12 @@ const pprof: Provider = { return [] } - // let workspaceRoot: string | undefined = undefined - // const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.parse(params.uri)) - // if (workspaceFolder && workspaceFolder !== null) { - // workspaceRoot = workspaceFolder.uri.path - // } - const searchDir = dirname(params.uri).replace(/^file:\/{2}/, '') const report = findReportPath(searchDir, { reportGlob: (settings.reportGlob as string) || '**/*.pb.gz', - // workspaceRoot: workspaceRoot, rootDirectoryMarkers: settings.rootDirectoryMarkers as string[], + // TODO: pass workspaceRoot once it's made available + // workspaceRoot: workspaceRoot, }) if (report === null) { return [] diff --git a/provider/pprof/package.json b/provider/pprof/package.json index 5a476b15..318430f3 100644 --- a/provider/pprof/package.json +++ b/provider/pprof/package.json @@ -16,14 +16,11 @@ "sideEffects": false, "scripts": { "build": "tsc --build", - "bundle": "tsc --build && esbuild --log-level=error --bundle --format=cjs --platform=node --external:vscode --outfile=dist/bundle.js index.ts", + "bundle": "tsc --build && esbuild --log-level=error --bundle --format=cjs --platform=node --outfile=dist/bundle.js index.ts", "prepublishOnly": "tsc --build --clean && pnpm run --silent build", "test": "vitest" }, "dependencies": { "@openctx/provider": "workspace:*" - }, - "devDependencies": { - "@types/vscode": "^1.85.0" } } From 89d9852f7375367398b8342362c08b653c558d1b Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Fri, 7 Jun 2024 11:58:08 +0200 Subject: [PATCH 12/29] ci: appease biome --- provider/pprof/_log.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/pprof/_log.ts b/provider/pprof/_log.ts index 3c2a928a..f6066bf6 100644 --- a/provider/pprof/_log.ts +++ b/provider/pprof/_log.ts @@ -4,7 +4,7 @@ * * This file is not imported anywhere, so no directories will be created on your machine * with `pprof` provider enabled. - * + * * It's only a temporary fixture -- there's probably a better solution to this. */ From 0233847990d4d0b4cca2edd4563addfda67fc32c Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Tue, 11 Jun 2024 14:41:41 +0200 Subject: [PATCH 13/29] feat(pprof): support standalone pprof Usually pprof is included in go distribution, but the extention should be able to use either of the tools, whichever one's installed. --- provider/pprof/pprof.test.ts | 45 ++++++++++++++++++++++++++++++++---- provider/pprof/pprof.ts | 20 ++++++++-------- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 31f663b5..a3c93912 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -1,6 +1,6 @@ import { execSync } from 'child_process' import { readdirSync } from 'fs' -import { beforeEach, describe, expect, test, vi } from 'vitest' +import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest' import { Pprof, type PprofTool, @@ -21,12 +21,27 @@ describe('pprof', () => { vi.clearAllMocks() }) - test('get pprof (installed)', () => { - execSyncMock.mockReturnValueOnce(buffer('/usr/local/go/bin')) + test('get pprof (go installed)', () => { + execSyncMock.mockImplementation(whichCommand('go', '/usr/local/go/bin/og', 'pprof')) + onTestFinished(() => { + execSyncMock.mockReset() + }) + + const pprof = getPprof() + + expect(execSync).toHaveBeenCalled() + expect(pprof).not.toBeNull() + }) + + test('get pprof (standalone pprof installed)', () => { + execSyncMock.mockImplementation(whichCommand('pprof', '/usr/local/go/bin/pprof', 'go')) + onTestFinished(() => { + execSyncMock.mockReset() + }) const pprof = getPprof() - expect(execSync).toHaveBeenCalledOnce() + expect(execSync).toHaveBeenCalled() expect(pprof).not.toBeNull() }) @@ -35,7 +50,7 @@ describe('pprof', () => { const pprof = getPprof() - expect(execSync).toHaveBeenCalledOnce() + expect(execSync).toHaveBeenCalled() expect(pprof).toBeNull() }) @@ -167,3 +182,23 @@ Showing top 3 nodes out of 7 function buffer(s: string): Buffer { return Buffer.from(s, 'utf-8') } + +/** + * whichCommand helper returns a mock implementation for `execSync` that expects some kind of lookup command, + * e.g. `which`, and returns "not found" for binaries that should not be found in the mock invocation. + * @param found name of the executable that is "found" in this mock + * @param foundPath executable path that should be returned + * @param notFound name of the executable that is "not found" in this mock + * @returns stdout buffer + */ +function whichCommand(found: string, foundPath: string, notFound: string): (cmd: string) => Buffer { + return (cmd: string): Buffer => { + switch (true) { + case cmd.includes(found): + return buffer(foundPath) + case cmd.includes(notFound): + return buffer(`${notFound} not found`) + } + return buffer('command not found: ' + cmd) + } +} diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index b7425d22..297ab8cd 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -25,21 +25,21 @@ const topNodeRegex = export type PprofTool = 'go tool pprof' | 'pprof' export function getPprof(): Pprof | null { - let hasGo = false - try { const stdout = execSync('which go').toString('utf-8').trim() if (!stdout.endsWith('not found')) { - hasGo = true + return new Pprof('go tool pprof') } - } catch (e) { - return null - } + } catch (e) {} - if (!hasGo) { - return null - } - return new Pprof('go tool pprof') + try { + const stdout = execSync('which pprof').toString('utf-8').trim() + if (!stdout.endsWith('not found')) { + return new Pprof('pprof') + } + } catch (e) {} + + return null } interface SearchOptions { From 75ccb130750549e4832b077af584103cd8dc7af0 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Tue, 11 Jun 2024 17:36:27 +0200 Subject: [PATCH 14/29] feat(pprof): pass binary to pprof if found --- provider/pprof/index.ts | 8 ++-- provider/pprof/pprof.test.ts | 73 +++++++++++++++++++++--------- provider/pprof/pprof.ts | 88 ++++++++++++++++++++++++++---------- 3 files changed, 120 insertions(+), 49 deletions(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index f942faca..567af275 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -9,7 +9,7 @@ import type { ProviderSettings, } from '@openctx/provider' import { escapeSpecial, parseGolang } from './parser.js' -import { type Node, findReportPath, getPprof } from './pprof.js' +import { type Node, findReportPath as findPprofSources, getPprof } from './pprof.js' /** * An [OpenCtx](https://openctx.org) provider that annotates every function declaration with @@ -38,16 +38,16 @@ const pprof: Provider = { } const searchDir = dirname(params.uri).replace(/^file:\/{2}/, '') - const report = findReportPath(searchDir, { + const sources = findPprofSources(searchDir, { reportGlob: (settings.reportGlob as string) || '**/*.pb.gz', rootDirectoryMarkers: settings.rootDirectoryMarkers as string[], // TODO: pass workspaceRoot once it's made available // workspaceRoot: workspaceRoot, }) - if (report === null) { + if (!sources.report) { return [] } - pprof.setReport(report) + pprof.setSources(sources) const content = parseGolang(params.content) if (!content) { diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index a3c93912..3ee97348 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -1,6 +1,6 @@ import { execSync } from 'child_process' -import { readdirSync } from 'fs' -import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest' +import { type Dirent, type PathLike, accessSync, readdirSync } from 'fs' +import { beforeEach, describe, expect, test, vi } from 'vitest' import { Pprof, type PprofTool, @@ -13,19 +13,21 @@ import { vi.mock('child_process', () => ({ execSync: vi.fn() })) const execSyncMock = vi.mocked(execSync) -vi.mock('fs', () => ({ readdirSync: vi.fn() })) +vi.mock('fs', async () => { + const fs = await vi.importActual('fs') + return { ...fs, readdirSync: vi.fn(), accessSync: vi.fn() } +}) const readdirSyncMock = vi.mocked(readdirSync) +const accessSyncMock = vi.mocked(accessSync) describe('pprof', () => { beforeEach(() => { vi.clearAllMocks() + vi.resetAllMocks() }) test('get pprof (go installed)', () => { execSyncMock.mockImplementation(whichCommand('go', '/usr/local/go/bin/og', 'pprof')) - onTestFinished(() => { - execSyncMock.mockReset() - }) const pprof = getPprof() @@ -35,9 +37,6 @@ describe('pprof', () => { test('get pprof (standalone pprof installed)', () => { execSyncMock.mockImplementation(whichCommand('pprof', '/usr/local/go/bin/pprof', 'go')) - onTestFinished(() => { - execSyncMock.mockReset() - }) const pprof = getPprof() @@ -59,20 +58,19 @@ describe('pprof', () => { return s === '/path/to' ? ['report.pprof'] : [] }) as unknown as typeof readdirSync) - const profilePath = findReportPath('/path/to/current', { + const sources = findReportPath('/path/to/current', { reportGlob: '*.pprof', workspaceRoot: '/path', }) - expect(profilePath).toBe('/path/to/report.pprof') + expect(sources.report).toBe('/path/to/report.pprof') }) test('find report with rootDirectoryMarkers (does not exist)', () => { readdirSyncMock.mockImplementation(((s: string): string[] => { switch (s) { case '/': - // Should not reach this case - return ['other.pprof'] + throw new Error('should not check after root directory') case '/root': // Contains root directory markers return ['README.md', '.git'] @@ -81,23 +79,55 @@ describe('pprof', () => { } }) as unknown as typeof readdirSync) - const profilePath = findReportPath('/root/to/current', { + const sources = findReportPath('/root/to/current', { reportGlob: '*.pprof', rootDirectoryMarkers: ['.git'], }) - expect(profilePath).toBeNull() + expect(sources.report).toBeUndefined() }) test('find report with workspaceRoot (does not exist)', () => { - readdirSyncMock.mockReturnValueOnce([]) + readdirSyncMock.mockReturnValue([]) - const profilePath = findReportPath('/path/to/current', { + const sources = findReportPath('/path/to/current', { reportGlob: '*.pprof', workspaceRoot: '/path', }) - expect(profilePath).toBeNull() + expect(sources.report).toBeUndefined() + }) + + test('find binary (exists in the same directory)', () => { + // File 'mybinary' exists in every directory, but only /root/mybinary/mybinary is executable + readdirSyncMock.mockReturnValue(['mybinary', 'test.yaml', 'README.md'] as unknown as Dirent[]) + accessSyncMock.mockImplementation((file: PathLike, mode?: number): void => { + if (!(file as string).endsWith('/mybinary')) { + throw new Error('not a binary') + } + }) + + const sources = findReportPath('/root/mybinary/is/here', { + reportGlob: '*.pprof', + workspaceRoot: '/root', + }) + + expect(sources.binary).toBe('/root/mybinary/mybinary') + expect(accessSync).toHaveBeenCalled() + }) + + test('find binary (matches binaryGlob)', () => { + readdirSyncMock.mockImplementation(((s: string): string[] => { + return s === '/root/cmd/here/comes' ? ['mybinary.exe', 'mybinary.yaml', 'nothing'] : [] + }) as unknown as typeof readdirSync) + + const sources = findReportPath('/root/cmd/here/comes/nothing', { + reportGlob: '*.pprof', + binaryGlob: '**/cmd/**/*.exe', + workspaceRoot: '/root', + }) + + expect(sources.binary).toBe('/root/cmd/here/comes/mybinary.exe') }) type TopCmdTest = { name: string; tool: PprofTool; opts: TopOptions; binary?: string; want: string } @@ -132,8 +162,7 @@ describe('pprof', () => { ])('top command ($name)', (tt: TopCmdTest) => { execSyncMock.mockReturnValueOnce(buffer('')) const pprof = new Pprof(tt.tool) - pprof.setReport('report.pprof') - pprof.setBinary(tt.binary) + pprof.setSources({ report: 'report.pprof', binary: tt.binary }) pprof.top(tt.opts) @@ -161,7 +190,7 @@ Showing top 3 nodes out of 7 execSyncMock.mockReturnValueOnce(buffer(outputCpu)) const pprof = new Pprof(tool) - pprof.setReport('/path/to/report.pprof') + pprof.setSources({ report: '/path/to/report.pprof' }) const top = pprof.top(topOptions) @@ -184,7 +213,7 @@ function buffer(s: string): Buffer { } /** - * whichCommand helper returns a mock implementation for `execSync` that expects some kind of lookup command, + * whichCommand helper returns a mock implementation for `execSync` that expects some kind of lookup command, * e.g. `which`, and returns "not found" for binaries that should not be found in the mock invocation. * @param found name of the executable that is "found" in this mock * @param foundPath executable path that should be returned diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 297ab8cd..71e33120 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -1,6 +1,6 @@ import { execSync } from 'child_process' -import { readdirSync } from 'fs' -import { dirname, join } from 'path' +import { constants, accessSync, readdirSync } from 'fs' +import { basename, dirname, join, parse } from 'path' import { matchGlob } from '@openctx/provider' /** @@ -46,12 +46,18 @@ interface SearchOptions { reportGlob: string workspaceRoot?: string rootDirectoryMarkers?: string[] - binaryGlob?: string // TODO: find binary if possible + binaryGlob?: string } -export function findReportPath(currentDir: string, options: SearchOptions): string | null { +/** PprofSources are passed to pprof command. */ +interface PprofSources { + report?: string + binary?: string +} + +export function findReportPath(currentDir: string, options: SearchOptions): PprofSources { const matchReport = matchGlob(options.reportGlob) - // const matchBinary = options.binaryGlob ? matchGlob(options.binaryGlob) : (s: string) => falses + const matchBinary = options.binaryGlob ? matchGlob(options.binaryGlob) : matchGoBinary const reachedRoot = (dir: string): boolean => { if (options.workspaceRoot !== undefined) { @@ -69,7 +75,7 @@ export function findReportPath(currentDir: string, options: SearchOptions): stri } } - let report: string | null = null + const sources: PprofSources = {} let searchDir = currentDir Search: while (true) { @@ -77,27 +83,65 @@ export function findReportPath(currentDir: string, options: SearchOptions): stri try { contents = readdirSync(searchDir) } catch (e) { - return null + break } for (const file of contents) { - // TODO: search for binary + const fullPath = join(searchDir, file) + if (matchBinary(fullPath)) { + sources.binary = fullPath + } + // Note, that by breaking the loop after finding the report we assume that the binary // is located in in the same directories or in one of the directories we've searched before. // Which is a rather fair assumption. + + // TODO: match full path to the file here, so that the user could limit directories + // where we should search for the report. if (matchReport(file)) { - report = join(searchDir, file) + sources.report = fullPath break Search } } if (reachedRoot(searchDir) || searchDir === '/') { - return null + break } searchDir = dirname(searchDir) } - return report + return sources +} + +/** + * matchGoBinary is a fallback matcher for finding Go binary to pass as a source to `pprof`. + * + * It uses a heuristic of relying on Go's convention to name binaries as their parent directories. + * For example, running `go build .` in `/project/cmd/thing` directory will produce `/project/cmd/thing/thing` binary. + * + * Go build automatically assigns "execute" permission to the binary, so `matchGoBinary` will only match executable files + * in order to differentiate from normal files that are called the same as their parent directory. + * @param file Full path to the file + * @returns + */ +function matchGoBinary(file: string): boolean { + const { base: name, ext, dir: dirFull } = parse(file) + const dir = basename(dirFull) + + if (name !== dir || ext !== '') { + return false + } + return isExecutable(file) +} + +/** isExecutable checks that a file has the "execute" permission. */ +function isExecutable(file: string): boolean { + try { + accessSync(file, constants.X_OK) + return true + } catch (err) { + return false + } } export interface TopOptions { @@ -143,24 +187,20 @@ export interface Node { */ export class Pprof { private tool: PprofTool - private report: string | null = null - private binary?: string + private sources: PprofSources constructor(tool: PprofTool) { this.tool = tool + this.sources = {} } - setReport(path: string) { - this.report = path - } - - setBinary(path?: string) { - this.binary = path + setSources(sources: PprofSources) { + this.sources = sources } // TODO: return raw output top(options: TopOptions): TopOutput | null { - if (!this.report) { + if (!this.sources.report) { return null } @@ -181,6 +221,8 @@ export class Pprof { } private topCmd(options: TopOptions): string { + const { report, binary } = this.sources + let cmd = this.tool + ` -top -show="${options.package}\\."` cmd += options.sort ? ` -${options.sort}` : ' -cum' @@ -188,10 +230,10 @@ export class Pprof { cmd += ' -noinlines' } - if (this.binary) { - cmd += ` ${this.binary}` + if (binary) { + cmd += ` ${binary}` } - cmd += ` ${this.report}` + cmd += ` ${report}` return cmd } From 517c52b0f1804c0d034c7ecf2177a1916f7af402 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Tue, 11 Jun 2024 17:55:16 +0200 Subject: [PATCH 15/29] feat(pprof): match reportGlob to a full file path This should allow users to specify directories where the pprof reports should and shouldn't be taken from. --- provider/pprof/pprof.test.ts | 20 +++++++++----------- provider/pprof/pprof.ts | 6 +++--- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 3ee97348..4fa245b6 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -54,12 +54,10 @@ describe('pprof', () => { }) test('find report (exists)', () => { - readdirSyncMock.mockImplementation(((s: string): string[] => { - return s === '/path/to' ? ['report.pprof'] : [] - }) as unknown as typeof readdirSync) + readdirSyncMock.mockReturnValue(['report.pprof'] as unknown as Dirent[]) const sources = findReportPath('/path/to/current', { - reportGlob: '*.pprof', + reportGlob: '**/to/*.pprof', workspaceRoot: '/path', }) @@ -71,7 +69,7 @@ describe('pprof', () => { switch (s) { case '/': throw new Error('should not check after root directory') - case '/root': + case '/path': // Contains root directory markers return ['README.md', '.git'] default: @@ -79,8 +77,8 @@ describe('pprof', () => { } }) as unknown as typeof readdirSync) - const sources = findReportPath('/root/to/current', { - reportGlob: '*.pprof', + const sources = findReportPath('/path/to/current', { + reportGlob: '**/*.pprof', rootDirectoryMarkers: ['.git'], }) @@ -88,10 +86,10 @@ describe('pprof', () => { }) test('find report with workspaceRoot (does not exist)', () => { - readdirSyncMock.mockReturnValue([]) + readdirSyncMock.mockReturnValue(['report.pprof'] as unknown as Dirent[]) const sources = findReportPath('/path/to/current', { - reportGlob: '*.pprof', + reportGlob: '/other/path/**/*.pprof', workspaceRoot: '/path', }) @@ -108,7 +106,7 @@ describe('pprof', () => { }) const sources = findReportPath('/root/mybinary/is/here', { - reportGlob: '*.pprof', + reportGlob: '**/*.pprof', workspaceRoot: '/root', }) @@ -122,7 +120,7 @@ describe('pprof', () => { }) as unknown as typeof readdirSync) const sources = findReportPath('/root/cmd/here/comes/nothing', { - reportGlob: '*.pprof', + reportGlob: '**/*.pprof', binaryGlob: '**/cmd/**/*.exe', workspaceRoot: '/root', }) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 71e33120..a4f21afe 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -95,10 +95,10 @@ export function findReportPath(currentDir: string, options: SearchOptions): Ppro // Note, that by breaking the loop after finding the report we assume that the binary // is located in in the same directories or in one of the directories we've searched before. // Which is a rather fair assumption. + // The search also favours the binary that's closest to the report file, + // as `sources.binary` will be overwritten with the more recent matches. - // TODO: match full path to the file here, so that the user could limit directories - // where we should search for the report. - if (matchReport(file)) { + if (matchReport(fullPath)) { sources.report = fullPath break Search } From 965822703c1950386b9b523cb000905355fd37a3 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 12 Jun 2024 14:25:56 +0200 Subject: [PATCH 16/29] fix(pprof): get units from `pprof -top` with >1 character --- provider/pprof/pprof.test.ts | 75 ++++++++++++++++++++++++++++-------- provider/pprof/pprof.ts | 6 +-- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 4fa245b6..c642e4d5 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -168,8 +168,15 @@ describe('pprof', () => { expect(execSync).toHaveBeenCalledWith(tt.want) }) - test('top (CPU)', () => { - const outputCpu = `File: pprof-example + type TopParseTest = { + name: string + stdout: string + want: TopOutput + } + test.each([ + { + name: 'cpu', + stdout: `File: pprof-example Type: cpu Time: Jun 1, 2024 at 10:56pm (CEST) Duration: 8.39s, Total samples = 13.02s (155.11%) @@ -181,28 +188,66 @@ Showing top 3 nodes out of 7 flat flat% sum% cum cum% 0.03s 0.23% 0.23% 6.38s 49.00% main.main 5.59s 42.93% 43.16% 6.21s 47.70% main.Run - 0.63s 4.84% 48.00% 0.63s 4.84% pkg/list.(*L).Init` - + 0.63s 4.84% 48.00% 0.63s 4.84% pkg/list.(*L).Init`, + want: { + file: 'pprof-example', + type: 'cpu', + unit: 's', + nodes: [ + { function: 'main.main', flat: 0.03, flatPerc: 0.23, cum: 6.38, cumPerc: 49 }, + { function: 'main.Run', flat: 5.59, flatPerc: 42.93, cum: 6.21, cumPerc: 47.7 }, + { + function: 'pkg/list.(*L).Init', + flat: 0.63, + flatPerc: 4.84, + cum: 0.63, + cumPerc: 4.84, + }, + ], + }, + }, + { + name: 'memory', + stdout: `File: pprof-example +Type: inuse_space +Time: May 31, 2024 at 7:35pm (CEST) +Active filters: + show=main\. +Showing nodes accounting for 116.43MB, 100% of 116.43MB total + flat flat% sum% cum cum% + 0.43MB 0.23% 0.23% 6.38MB 49.00% main.main + 77.4MB 42.93% 43.16% 6.21MB 47.70% main.Run + 10.0MB 4.84% 48.00% 10.0MB 4.84% pkg/list.(*L).Init`, + want: { + file: 'pprof-example', + type: 'inuse_space', + unit: 'MB', + nodes: [ + { function: 'main.main', flat: 0.43, flatPerc: 0.23, cum: 6.38, cumPerc: 49 }, + { function: 'main.Run', flat: 77.4, flatPerc: 42.93, cum: 6.21, cumPerc: 47.7 }, + { + function: 'pkg/list.(*L).Init', + flat: 10.0, + flatPerc: 4.84, + cum: 10.0, + cumPerc: 4.84, + }, + ], + }, + }, + ])('parsing top output ($name)', (tt: TopParseTest) => { const tool: PprofTool = 'go tool pprof' const topOptions: TopOptions = { package: 'main' } - execSyncMock.mockReturnValueOnce(buffer(outputCpu)) + execSyncMock.mockReturnValueOnce(buffer(tt.stdout)) const pprof = new Pprof(tool) pprof.setSources({ report: '/path/to/report.pprof' }) const top = pprof.top(topOptions) - expect(top).toStrictEqual({ - file: 'pprof-example', - type: 'cpu', - unit: 's', - nodes: [ - { function: 'main.main', flat: 0.03, flatPerc: 0.23, cum: 6.38, cumPerc: 49 }, - { function: 'main.Run', flat: 5.59, flatPerc: 42.93, cum: 6.21, cumPerc: 47.7 }, - { function: 'pkg/list.(*L).Init', flat: 0.63, flatPerc: 4.84, cum: 0.63, cumPerc: 4.84 }, - ], - }) + expect(top).toStrictEqual(tt.want) }) + test.todo('list') }) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index a4f21afe..fe49c61e 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -13,10 +13,10 @@ import { matchGlob } from '@openctx/provider' * 5.59s 42.93% 43.16% 6.21s 47.70% container/list.(*List).Init * ``` * - * @see https://regex101.com/r/26v9rd/1 + * @see https://regex101.com/r/hP9plv/1 */ const topNodeRegex = - /^ +(?[\d\\.]+)(?\w) +(?[\d\\.]+)% +(?:[\d\\.]+)% +(?[\d\\.]+)(?:\w) +(?[\d\\.]+)% +(?[\w\\.\\(\\*\\)\\/]+)/ + /^ +(?[\d\\.]+)(?\w+) +(?[\d\\.]+)% +(?:[\d\\.]+)% +(?[\d\\.]+)(?:\w+) +(?[\d\\.]+)% +(?[\w\\.\\(\\*\\)\\/]+)/ /** * PprofTool is a CLI tool for reading and visualizing profile reports. @@ -271,7 +271,7 @@ export class Pprof { } return { - type: reportType ? reportType[1] || 'cpu' : 'cpu', + type: reportType ? reportType[1] || 'cpu' : '', file: binaryName ? binaryName[1] : undefined, unit: unit || 's', nodes: nodes, From 478ca167fc6a7040d84999d5bf831696b27f6615 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 12 Jun 2024 16:32:17 +0200 Subject: [PATCH 17/29] feat(pprof): pass detailed breakdown (pprof list) to "ai" field in annotation's item --- provider/pprof/index.ts | 27 ++++++++++----- provider/pprof/parser.test.ts | 10 +++--- provider/pprof/parser.ts | 13 ++------ provider/pprof/pprof.test.ts | 29 +++++++++++++++- provider/pprof/pprof.ts | 63 ++++++++++++++++++++++++++++++++++- 5 files changed, 116 insertions(+), 26 deletions(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index 567af275..68590529 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -3,12 +3,13 @@ import type { Annotation, AnnotationsParams, AnnotationsResult, + Item, MetaParams, MetaResult, Provider, ProviderSettings, } from '@openctx/provider' -import { escapeSpecial, parseGolang } from './parser.js' +import { parseGolang } from './parser.js' import { type Node, findReportPath as findPprofSources, getPprof } from './pprof.js' /** @@ -61,21 +62,31 @@ const pprof: Provider = { const anns: Annotation[] = [] - // TODO(1): turn Func[] into a Record for faster lookups. - // TODO(2): do not escape pprofRegex, delay it until it's used in `pprof -list`. - // This way we do not need to do the awkward conversion of `node.function` to match it. + // TODO(1): turn Func[] into a Record for faster lookups (top output should be ordered, funcs don't need to). for (const func of content.funcs) { top.nodes.forEach((node: Node, i: number) => { - if (func.pprofRegex !== escapeSpecial(node.function)) { + if (func.pprofRegex !== node.function) { return } + let item: Item = { + title: `[#${i + 1}] ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`, + } + + const list = pprof.list(node.function) + if (list) { + item = { + ...item, + ai: { + content: "Output of 'pprof -list' command for this function:\n" + list.raw, + }, + } + } + anns.push({ uri: params.uri, range: func.range, - item: { - title: `[#${i + 1}] CPU Time: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`, - }, + item: item, }) }) } diff --git a/provider/pprof/parser.test.ts b/provider/pprof/parser.test.ts index 3dd28194..65da66cf 100644 --- a/provider/pprof/parser.test.ts +++ b/provider/pprof/parser.test.ts @@ -25,28 +25,28 @@ func (t Thing) String() string { return "thing" } { name: 'A', range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } }, - pprofRegex: 'example\\.A', + pprofRegex: 'example.A', }, { name: 'b_func', range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } }, - pprofRegex: 'example\\.b_func', + pprofRegex: 'example.b_func', }, { name: 'A2', range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } }, - pprofRegex: 'example\\.A2', + pprofRegex: 'example.A2', }, { name: 'doStuff', range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } }, - pprofRegex: 'example\\.\\(\\*Thing\\)\\.doStuff', + pprofRegex: 'example.(*Thing).doStuff', receiver: '*Thing', }, { name: 'String', range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } }, - pprofRegex: 'example\\.\\(Thing\\)\\.String', + pprofRegex: 'example.(Thing).String', receiver: 'Thing', }, ], diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts index 10f42fa1..954b3ce3 100644 --- a/provider/pprof/parser.ts +++ b/provider/pprof/parser.ts @@ -57,7 +57,7 @@ export function parseGolang(source: string): Contents | null { start: { line: i, character: start }, end: { line: i, character: end }, }, - pprofRegex: escapeSpecial(`${pkg}.${func}`), + pprofRegex: `${pkg}.${func}`, }) break } @@ -87,7 +87,7 @@ export function parseGolang(source: string): Contents | null { start: { line: i, character: start }, end: { line: i, character: end }, }, - pprofRegex: escapeSpecial(`${pkg}.(${receiver}).${func}`), + pprofRegex: `${pkg}.(${receiver}).${func}`, receiver: receiver, }) break @@ -97,12 +97,3 @@ export function parseGolang(source: string): Contents | null { return result } - -/** - * Escape all special regex characters in a string. - * @param s string - * @returns string - */ -export function escapeSpecial(s: string): string { - return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&') -} diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index c642e4d5..aed21822 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -2,6 +2,7 @@ import { execSync } from 'child_process' import { type Dirent, type PathLike, accessSync, readdirSync } from 'fs' import { beforeEach, describe, expect, test, vi } from 'vitest' import { + type ListOutput, Pprof, type PprofTool, type TopOptions, @@ -248,7 +249,33 @@ Showing nodes accounting for 116.43MB, 100% of 116.43MB total expect(top).toStrictEqual(tt.want) }) - test.todo('list') + test('list', () => { + const stdout = `Total: 116.43MB +ROUTINE ======================== main.buildDiamond in /Users/johndoe/go/src/local/pprof-example/main.go + 0 49.57MB (flat, cum) 42.57% of Total + . . 108:func buildDiamond(cfgraph *CFG, start int) int { + . . 109: bb0 := start + . 11.50MB 110: NewBasicBlockEdge(cfgraph, bb0, bb0+1) + . 21.06MB 111: NewBasicBlockEdge(cfgraph, bb0, bb0+2) + . 13.50MB 112: NewBasicBlockEdge(cfgraph, bb0+1, bb0+3) + . 3.50MB 113: NewBasicBlockEdge(cfgraph, bb0+2, bb0+3) + . . 114: + . . 115: return bb0 + 3 + . . 116:} + . . 117: + . . 118:func buildConnect(cfgraph *CFG, start int, end int) {` + execSyncMock.mockReturnValueOnce(buffer(stdout)) + const pprof = new Pprof('pprof') + pprof.setSources({ report: 'report.mprof', binary: './mybinary' }) + + const list = pprof.list('example.(*Thing).Do') + + expect(execSyncMock).toHaveBeenCalledOnce() + expect(execSync).toHaveBeenCalledWith( + `pprof -list "example\\.\\(\\*Thing\\)\\.Do" ./mybinary report.mprof` + ) + expect(list).toStrictEqual({ raw: stdout }) + }) }) function buffer(s: string): Buffer { diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index fe49c61e..fdadd955 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -182,6 +182,10 @@ export interface Node { cumPerc: number } +export interface ListOutput { + raw: string +} + /** * Pprof is a wrapper for working with `pprof` CLI. */ @@ -213,7 +217,11 @@ export class Pprof { return null } - if (out === null || out.includes('no such file or directory')) { + if ( + out === null || + out.includes('no such file or directory') || + out.includes('Show expression matched no samples') + ) { return null } @@ -230,6 +238,8 @@ export class Pprof { cmd += ' -noinlines' } + // Standalone `pprof` is not able to parse a Go binary, so it ignores it altogether. + // Should we omit it from the command in case this.tool === 'pprof' ? if (binary) { cmd += ` ${binary}` } @@ -277,4 +287,55 @@ export class Pprof { nodes: nodes, } } + + /** + * list fetches the detailed line-by-line breakdown of the function's resource consumption. + * @param funcRegex fully-qualified function name. That includes both the package name and the name of the struct if the function is a method. + * E.g. `example.MyFunction` or `example.(*ReceiverStruct).MyMethod`. + * @returns raw `-list` output + */ + public list(funcRegex: string): ListOutput | null { + if (!this.sources.report) { + return null + } + + const cmd = this.listCmd(funcRegex) + + let out: string | null = null + try { + out = execSync(cmd).toString('utf-8').trim() + } catch (e) { + return null + } + + if ( + out === null || + out.includes('no such file or directory') || + out.includes('no matches found for regexp') + ) { + return null + } + return { raw: out } + } + + private listCmd(funcRegex: string): string { + const { report, binary } = this.sources + + let cmd = this.tool + ` -list "${escapeSpecial(funcRegex)}"` + if (binary) { + cmd += ` ${binary}` + } + cmd += ` ${report}` + + return cmd + } +} + +/** + * Escape all special regex characters in a string. + * @param s string + * @returns string + */ +function escapeSpecial(s: string): string { + return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&') } From 063257ac4a823e1feab670b905fb9af053fd152d Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Wed, 12 Jun 2024 16:52:34 +0200 Subject: [PATCH 18/29] refactor(pprof): store file's functions as a Record to minimize looping In the `annotations()` we would previously loop over every function in a file, then loop over the function from `pprof top` to find information for the given function. Since "top" results should remain ordered, it seems easier to return funcs as a Record --- provider/pprof/index.ts | 46 ++++++++++++++++------------------- provider/pprof/parser.test.ts | 19 ++++++--------- provider/pprof/parser.ts | 19 ++++++++------- provider/pprof/pprof.ts | 5 +++- 4 files changed, 42 insertions(+), 47 deletions(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index 68590529..ffadf3f3 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -61,36 +61,32 @@ const pprof: Provider = { } const anns: Annotation[] = [] + top.nodes.forEach((node: Node, i: number) => { + const func = content.funcs[node.function] + if (!func) { + return + } - // TODO(1): turn Func[] into a Record for faster lookups (top output should be ordered, funcs don't need to). - for (const func of content.funcs) { - top.nodes.forEach((node: Node, i: number) => { - if (func.pprofRegex !== node.function) { - return - } - - let item: Item = { - title: `[#${i + 1}] ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`, - } + let item: Item = { + title: `[#${i + 1}] ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`, + } - const list = pprof.list(node.function) - if (list) { - item = { - ...item, - ai: { - content: "Output of 'pprof -list' command for this function:\n" + list.raw, - }, - } + const list = pprof.list(node.function) + if (list) { + item = { + ...item, + ai: { + content: "Output of 'pprof -list' command for this function:\n" + list.raw, + }, } + } - anns.push({ - uri: params.uri, - range: func.range, - item: item, - }) + anns.push({ + uri: params.uri, + range: func.range, + item: item, }) - } - + }) return anns }, } diff --git a/provider/pprof/parser.test.ts b/provider/pprof/parser.test.ts index 65da66cf..c234a33e 100644 --- a/provider/pprof/parser.test.ts +++ b/provider/pprof/parser.test.ts @@ -21,35 +21,30 @@ func (t Thing) String() string { return "thing" } expect(parseGolang(content)).toStrictEqual({ package: 'example', - funcs: [ - { + funcs: { + 'example.A': { name: 'A', range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } }, - pprofRegex: 'example.A', }, - { + 'example.b_func': { name: 'b_func', range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } }, - pprofRegex: 'example.b_func', }, - { + 'example.A2': { name: 'A2', range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } }, - pprofRegex: 'example.A2', }, - { + 'example.(*Thing).doStuff': { name: 'doStuff', range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } }, - pprofRegex: 'example.(*Thing).doStuff', receiver: '*Thing', }, - { + 'example.(Thing).String': { name: 'String', range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } }, - pprofRegex: 'example.(Thing).String', receiver: 'Thing', }, - ], + }, }) }) }) diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts index 954b3ce3..2b103944 100644 --- a/provider/pprof/parser.ts +++ b/provider/pprof/parser.ts @@ -6,14 +6,17 @@ const methodRegex = /^func \(\w+ (\*)?(\w+)\) (\w+)(?:\()/m export interface Contents { package: string - funcs: Func[] + + /** The key for each function is its fully-qualified name, e.g. example.MyFunc or example.(*Thing).MyMethod, + * as they are unique within the file. + */ + funcs: Record } /** Func is a Go function or method with additional metadata to help locate it in the file and filter for in in `pprof`. */ export interface Func { name: string range: Range - pprofRegex: string receiver?: string } @@ -25,7 +28,7 @@ export function parseGolang(source: string): Contents | null { const pkg = pkgMatch[1] const result: Contents = { package: pkg, - funcs: [], + funcs: {}, } const lines = source.split('\n') @@ -51,14 +54,13 @@ export function parseGolang(source: string): Contents | null { const start = 5 // "func ".length const { func, end } = readFuncName(start) - result.funcs.push({ + result.funcs[`${pkg}.${func}`] = { name: func, range: { start: { line: i, character: start }, end: { line: i, character: end }, }, - pprofRegex: `${pkg}.${func}`, - }) + } break } case methodRegex.test(line): { @@ -81,15 +83,14 @@ export function parseGolang(source: string): Contents | null { const start = rparen + 2 const { func, end } = readFuncName(start) - result.funcs.push({ + result.funcs[`${pkg}.(${receiver}).${func}`] = { name: func, range: { start: { line: i, character: start }, end: { line: i, character: end }, }, - pprofRegex: `${pkg}.(${receiver}).${func}`, receiver: receiver, - }) + } break } } diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index fdadd955..abbf76f0 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -202,7 +202,6 @@ export class Pprof { this.sources = sources } - // TODO: return raw output top(options: TopOptions): TopOutput | null { if (!this.sources.report) { return null @@ -271,6 +270,10 @@ export class Pprof { unit = node.unit } + // Should we include the raw output here the way we do in list()? + // It may include entries for other functions in the same package + // which are declared in different files, which might be a useful + // information to Cody. nodes.push({ function: node.func, flat: parseFloat(node.flat), From 63526e340a48bd2a5495d7e26ea62d8ba3b5eb92 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 00:50:16 +0200 Subject: [PATCH 19/29] chore(pprof): change annotation title --- provider/pprof/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index ffadf3f3..62310898 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -68,7 +68,7 @@ const pprof: Provider = { } let item: Item = { - title: `[#${i + 1}] ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`, + title: `pprof ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (#${i + 1}, cum)`, } const list = pprof.list(node.function) From 7b4acab1718adafd096df6ca29271bd93d393158 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 01:58:17 +0200 Subject: [PATCH 20/29] feat(pprof): apply all provider settings Previously reportGlob, binaryGlob, and all TopOptions were ignored even if set in settings.json. --- provider/pprof/index.ts | 59 ++++++++++++++++++++++++++++++------ provider/pprof/pprof.test.ts | 6 ++++ provider/pprof/pprof.ts | 19 +++++++++--- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index 62310898..f6cb8df5 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -7,17 +7,55 @@ import type { MetaParams, MetaResult, Provider, - ProviderSettings, } from '@openctx/provider' import { parseGolang } from './parser.js' -import { type Node, findReportPath as findPprofSources, getPprof } from './pprof.js' +import { type Node, type TopOptions, findReportPath as findPprofSources, getPprof } from './pprof.js' + +interface Settings { + /** + * Glob pattern to match the profile report. + * + * Note, that forward slashes _do not need_ to be escaped in the patterns provided in `settings.json` + * + * @default "**\/*.pprof" + * @example "**\/cmd\/*.pb.gz" (limit to asubdirectory) + */ + reportGlob?: string + + /** + * Glob pattern to match the Go binary from which the report was generated. + * + * By default `binaryGlob` not set. The provider will try to locate it by searching + * for an executable file whose name matches that of its parent directory. + * This is what a binary produces by `go build .` would be conventionally named. + */ + binaryGlob?: string + + /** + * The provider will not traverse the file tree past the directory containing `rootDirectoryMarkers`, + * when searching for the profile report and the binary. + * + * @default [".git", "go.mod"] + */ + rootDirectoryMarkers?: string[] + + /** + * Options to control `pprof -top` output. + * + * @default top: { excludeInline: true, sort: 'cum' } + * @example top: { excludeInline: false, sort: 'flat', nodeCount: 10 } + */ + top?: Pick +} /** * An [OpenCtx](https://openctx.org) provider that annotates every function declaration with * the CPU time and memory allocations associated with it. + * + * Only Go files are supported. */ -const pprof: Provider = { - meta(params: MetaParams, settings: ProviderSettings): MetaResult { +const pprof: Provider = { + meta(params: MetaParams, settings: Settings): MetaResult { return { name: 'pprof', annotations: { @@ -26,7 +64,7 @@ const pprof: Provider = { } }, - annotations(params: AnnotationsParams, settings: ProviderSettings): AnnotationsResult { + annotations(params: AnnotationsParams, settings: Settings): AnnotationsResult { // Test files do not need pprof annotations. if (params.uri.endsWith('_test.go')) { return [] @@ -40,8 +78,9 @@ const pprof: Provider = { const searchDir = dirname(params.uri).replace(/^file:\/{2}/, '') const sources = findPprofSources(searchDir, { - reportGlob: (settings.reportGlob as string) || '**/*.pb.gz', - rootDirectoryMarkers: settings.rootDirectoryMarkers as string[], + reportGlob: settings.reportGlob || '**/*.pprof', + rootDirectoryMarkers: settings.rootDirectoryMarkers || ['.git', 'go.mod'], + binaryGlob: settings.binaryGlob, // TODO: pass workspaceRoot once it's made available // workspaceRoot: workspaceRoot, }) @@ -55,7 +94,7 @@ const pprof: Provider = { return [] } - const top = pprof.top({ package: content.package }) + const top = pprof.top({ ...settings.top, package: content.package }) if (top === null) { return [] } @@ -68,7 +107,9 @@ const pprof: Provider = { } let item: Item = { - title: `pprof ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (#${i + 1}, cum)`, + title: `pprof ${top.type}: cum ${node.cum}${top.unit}, ${node.cumPerc}% (#${ + i + 1 + }, sort=${settings.top?.sort || 'cum'})`, } const list = pprof.list(node.function) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index aed21822..90ad60cb 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -158,6 +158,12 @@ describe('pprof', () => { opts: { package: 'main', excludeInline: false }, want: `go tool pprof -top -show="main\\." -cum report.pprof`, }, + { + name: 'limit node count', + tool: 'go tool pprof', + opts: { package: 'main', nodeCount: 2 }, + want: `go tool pprof -top -show="main\\." -cum -noinlines -nodecount=2 report.pprof`, + }, ])('top command ($name)', (tt: TopCmdTest) => { execSyncMock.mockReturnValueOnce(buffer('')) const pprof = new Pprof(tt.tool) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index abbf76f0..a96f18d5 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -183,6 +183,7 @@ export interface Node { } export interface ListOutput { + /** Raw output of `pprof -list`. */ raw: string } @@ -229,14 +230,22 @@ export class Pprof { private topCmd(options: TopOptions): string { const { report, binary } = this.sources + const opt: TopOptions = { + ...options, + sort: options.sort ?? 'cum', + excludeInline: options.excludeInline ?? true, + } - let cmd = this.tool + ` -top -show="${options.package}\\."` - cmd += options.sort ? ` -${options.sort}` : ' -cum' + let cmd = this.tool + ` -top -show="${options.package}\\." -${opt.sort}` - if (options.excludeInline === undefined || options.excludeInline === true) { + if (opt.excludeInline) { cmd += ' -noinlines' } + if (options.nodeCount) { + cmd += ` -nodecount=${options.nodeCount}` + } + // Standalone `pprof` is not able to parse a Go binary, so it ignores it altogether. // Should we omit it from the command in case this.tool === 'pprof' ? if (binary) { @@ -284,9 +293,9 @@ export class Pprof { } return { - type: reportType ? reportType[1] || 'cpu' : '', + type: reportType ? reportType[1] ?? 'cpu' : '', file: binaryName ? binaryName[1] : undefined, - unit: unit || 's', + unit: unit ?? 's', nodes: nodes, } } From 2afee106bfe940961cde06170848e603afd596d2 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 02:10:49 +0200 Subject: [PATCH 21/29] chore: appease biome --- provider/pprof/parser.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/provider/pprof/parser.ts b/provider/pprof/parser.ts index 2b103944..33475e1c 100644 --- a/provider/pprof/parser.ts +++ b/provider/pprof/parser.ts @@ -6,10 +6,10 @@ const methodRegex = /^func \(\w+ (\*)?(\w+)\) (\w+)(?:\()/m export interface Contents { package: string - + /** The key for each function is its fully-qualified name, e.g. example.MyFunc or example.(*Thing).MyMethod, * as they are unique within the file. - */ + */ funcs: Record } From 3350351daa4fa74e2eaf3ceb56ec10ad7ab2a445 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 10:46:46 +0200 Subject: [PATCH 22/29] build(pprof): bundle to esm format --- provider/pprof/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/pprof/package.json b/provider/pprof/package.json index 318430f3..63854195 100644 --- a/provider/pprof/package.json +++ b/provider/pprof/package.json @@ -16,7 +16,7 @@ "sideEffects": false, "scripts": { "build": "tsc --build", - "bundle": "tsc --build && esbuild --log-level=error --bundle --format=cjs --platform=node --outfile=dist/bundle.js index.ts", + "bundle": "tsc --build && esbuild --log-level=error --bundle --format=esm --platform=node --outfile=dist/bundle.js index.ts", "prepublishOnly": "tsc --build --clean && pnpm run --silent build", "test": "vitest" }, From 137bb1a230ad165ad5951c80acdda4db2dd6c7a9 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 13:12:33 +0200 Subject: [PATCH 23/29] fix(pprof): avoid breaking search loop prematurely When searching for the Go binary, we assume that it will be in the same directory as the report, or in one of its child directories. If we break the loop the moment we find the report, then we might not find the binary even if it's in the same directory but is alphabetically sorted after the report file. --- provider/pprof/pprof.test.ts | 11 +++++++++-- provider/pprof/pprof.ts | 20 +++++++++----------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index 90ad60cb..f1e243cd 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -98,8 +98,15 @@ describe('pprof', () => { }) test('find binary (exists in the same directory)', () => { - // File 'mybinary' exists in every directory, but only /root/mybinary/mybinary is executable - readdirSyncMock.mockReturnValue(['mybinary', 'test.yaml', 'README.md'] as unknown as Dirent[]) + readdirSyncMock.mockImplementation(((s: string): string[] => { + switch (s) { + case '/root/mybinary': + return ['report.pprof', 'mybinary'] + default: + // File 'mybinary' exists in every directory, but only /root/mybinary/mybinary is executable + return ['README.md', 'mybinary'] + } + }) as unknown as typeof readdirSync) accessSyncMock.mockImplementation((file: PathLike, mode?: number): void => { if (!(file as string).endsWith('/mybinary')) { throw new Error('not a binary') diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index a96f18d5..c37ee264 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -78,7 +78,7 @@ export function findReportPath(currentDir: string, options: SearchOptions): Ppro const sources: PprofSources = {} let searchDir = currentDir - Search: while (true) { + while (true) { let contents: string[] = [] try { contents = readdirSync(searchDir) @@ -88,23 +88,21 @@ export function findReportPath(currentDir: string, options: SearchOptions): Ppro for (const file of contents) { const fullPath = join(searchDir, file) + if (!sources.report && matchReport(fullPath)) { + sources.report = fullPath + } + + // The search favours the binary that's closest to the report file, + // as `sources.binary` will be overwritten with the more recent matches. if (matchBinary(fullPath)) { sources.binary = fullPath } + } // Note, that by breaking the loop after finding the report we assume that the binary // is located in in the same directories or in one of the directories we've searched before. // Which is a rather fair assumption. - // The search also favours the binary that's closest to the report file, - // as `sources.binary` will be overwritten with the more recent matches. - - if (matchReport(fullPath)) { - sources.report = fullPath - break Search - } - } - - if (reachedRoot(searchDir) || searchDir === '/') { + if (sources.report || reachedRoot(searchDir) || searchDir === '/') { break } searchDir = dirname(searchDir) From 519daef8604afac22b2aea44949a9e692a1242a0 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 13:27:51 +0200 Subject: [PATCH 24/29] ci: fix formatting --- provider/pprof/pprof.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index c37ee264..07433379 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -97,11 +97,11 @@ export function findReportPath(currentDir: string, options: SearchOptions): Ppro if (matchBinary(fullPath)) { sources.binary = fullPath } - } + } - // Note, that by breaking the loop after finding the report we assume that the binary - // is located in in the same directories or in one of the directories we've searched before. - // Which is a rather fair assumption. + // Note, that by breaking the loop after finding the report we assume that the binary + // is located in in the same directories or in one of the directories we've searched before. + // Which is a rather fair assumption. if (sources.report || reachedRoot(searchDir) || searchDir === '/') { break } From 28f4f5406f54545164310675c62bd001c2645ea6 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 13 Jun 2024 23:20:12 +0200 Subject: [PATCH 25/29] chore(pprof): write docs --- provider/pprof/README.md | 51 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/provider/pprof/README.md b/provider/pprof/README.md index c8989254..3b34b798 100644 --- a/provider/pprof/README.md +++ b/provider/pprof/README.md @@ -1,10 +1,19 @@ # [pprof](https://github.com/google/pprof) context provider for OpenCtx -This is a context provider for [OpenCtx](https://openctx.org) that annotates functions with the CPU time and memory allocations attributed to them. +[OpenCtx](https://openctx.org) provider that annotates Go functions with their associated CPU time and memory allocations based on the CPU/memory profiles. + +As profiling reports are usually not stored in a centralized remote location (like, e.g. docs or logs) and only exist on your machine, this provider only supports local VSCode client. It also does not provide annotations for test files. + +When enabled, pprof provider will: + +1. Search the workspace to find a profiling report and, optionally, a Go binary that produced it. +1. Get `pprof -top` nodes for the current package. +1. Create an annotation for each function/method in the current file denoting its resourse consumption. +1. Pass a detailed `pprof -list` breakdown to `annotation.item.ai` to be consumed by Cody. ## Usage -Add the following to your settings in any OpenCtx client: +Add the following to your `settings.json`: ```json "openctx.providers": { @@ -13,12 +22,48 @@ Add the following to your settings in any OpenCtx client: }, ``` +Pprof provider has reasonable defaults, so no additional configuration in necessary if you follow the standard naming conventions for pprof reports and Go binaries, e.g. that a cpu profile report has `.pprof` extension. + +Most of the time, however, you'll want to adjust the config to suit your preferences. + ## Configuration -> TODO +The default configuration looks like this: + +```json +{ + "reportGlob": "**/*.pprof", + "binaryGlob": undefined, // By default, looks for a binary whose name matches the name of its parent directory + "rootDirectoryMarkers": ["go.mod", ".git"], + "top": { // Options to control `pprof -top` output + "excludeInline": true, // Add `-noinlines` + "nodeCount": undefined, // Add `-nodecount=x`, not set by default + "sort": "cum" // Set `-cum` or `-flat` + } +} +``` + +## Limitations + +`pprof` can collect stack traces for a number of [different profiles](https://pkg.go.dev/runtime/pprof#Profile): + +``` +goroutine - stack traces of all current goroutines +heap - a sampling of memory allocations of live objects +allocs - a sampling of all past memory allocations +threadcreate - stack traces that led to the creation of new OS threads +block - stack traces that led to blocking on synchronization primitives +mutex - stack traces of holders of contended mutexes +``` + +This provider only supports `heap` and CPU profile[^1]. ## Development - [Source code](https://sourcegraph.com/github.com/sourcegraph/openctx/-/tree/provider/pprof) - [Docs](https://openctx.org/docs/providers/pprof) - License: Apache 2.0 + +____ + +[^1]: The CPU profile is not available as a `runtime/pprof.Profile` and has a special API. From 3bdc9860fd55c87ef788ed939a6a1aeafd02a9e2 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 12 Dec 2024 14:43:35 +0100 Subject: [PATCH 26/29] ci: appease biomi --- provider/pprof/_log.ts | 4 ++-- provider/pprof/index.test.ts | 4 ++-- provider/pprof/index.ts | 7 +++---- provider/pprof/pprof.test.ts | 6 +++--- provider/pprof/pprof.ts | 18 +++++++++--------- provider/pprof/tsconfig.json | 4 ++-- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/provider/pprof/_log.ts b/provider/pprof/_log.ts index f6066bf6..ce0eaf65 100644 --- a/provider/pprof/_log.ts +++ b/provider/pprof/_log.ts @@ -8,8 +8,8 @@ * It's only a temporary fixture -- there's probably a better solution to this. */ -import { appendFileSync, closeSync, mkdirSync, openSync, statSync } from 'fs' -import { join } from 'path' +import { appendFileSync, closeSync, mkdirSync, openSync, statSync } from 'node:fs' +import { join } from 'node:path' const logDir = `${process.env.HOME}/log/openctx` const logFile = join(logDir, 'provider-pprof.log') diff --git a/provider/pprof/index.test.ts b/provider/pprof/index.test.ts index f5d209ee..063d5594 100644 --- a/provider/pprof/index.test.ts +++ b/provider/pprof/index.test.ts @@ -41,8 +41,8 @@ describe('pprof', () => { uri: '/pkg/thing_test.go', content: content, }, - {} - ) + {}, + ), ).toHaveLength(0) expect(getPprofMock).not.toBeCalled() }) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index f6cb8df5..ac9ea8cd 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -1,4 +1,4 @@ -import { dirname } from 'path' +import { dirname } from 'node:path' import type { Annotation, AnnotationsParams, @@ -107,9 +107,8 @@ const pprof: Provider = { } let item: Item = { - title: `pprof ${top.type}: cum ${node.cum}${top.unit}, ${node.cumPerc}% (#${ - i + 1 - }, sort=${settings.top?.sort || 'cum'})`, + title: `pprof ${top.type}: cum ${node.cum}${top.unit}, ${node.cumPerc}% (#${i + 1 + }, sort=${settings.top?.sort || 'cum'})`, } const list = pprof.list(node.function) diff --git a/provider/pprof/pprof.test.ts b/provider/pprof/pprof.test.ts index f1e243cd..862f431d 100644 --- a/provider/pprof/pprof.test.ts +++ b/provider/pprof/pprof.test.ts @@ -1,5 +1,5 @@ -import { execSync } from 'child_process' -import { type Dirent, type PathLike, accessSync, readdirSync } from 'fs' +import { execSync } from 'node:child_process' +import { type Dirent, type PathLike, accessSync, readdirSync } from 'node:fs' import { beforeEach, describe, expect, test, vi } from 'vitest' import { type ListOutput, @@ -285,7 +285,7 @@ ROUTINE ======================== main.buildDiamond in /Users/johndoe/go/src/loca expect(execSyncMock).toHaveBeenCalledOnce() expect(execSync).toHaveBeenCalledWith( - `pprof -list "example\\.\\(\\*Thing\\)\\.Do" ./mybinary report.mprof` + `pprof -list "example\\.\\(\\*Thing\\)\\.Do" ./mybinary report.mprof`, ) expect(list).toStrictEqual({ raw: stdout }) }) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 07433379..d728776c 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -1,6 +1,6 @@ -import { execSync } from 'child_process' -import { constants, accessSync, readdirSync } from 'fs' -import { basename, dirname, join, parse } from 'path' +import { execSync } from 'node:child_process' +import { constants, accessSync, readdirSync } from 'node:fs' +import { basename, dirname, join, parse } from 'node:path' import { matchGlob } from '@openctx/provider' /** @@ -30,14 +30,14 @@ export function getPprof(): Pprof | null { if (!stdout.endsWith('not found')) { return new Pprof('go tool pprof') } - } catch (e) {} + } catch (e) { } try { const stdout = execSync('which pprof').toString('utf-8').trim() if (!stdout.endsWith('not found')) { return new Pprof('pprof') } - } catch (e) {} + } catch (e) { } return null } @@ -283,10 +283,10 @@ export class Pprof { // information to Cody. nodes.push({ function: node.func, - flat: parseFloat(node.flat), - flatPerc: parseFloat(node.flatPerc), - cum: parseFloat(node.cum), - cumPerc: parseFloat(node.cumPerc), + flat: Number.parseFloat(node.flat), + flatPerc: Number.parseFloat(node.flatPerc), + cum: Number.parseFloat(node.cum), + cumPerc: Number.parseFloat(node.cumPerc), }) } diff --git a/provider/pprof/tsconfig.json b/provider/pprof/tsconfig.json index 4ff7ab81..a1d94187 100644 --- a/provider/pprof/tsconfig.json +++ b/provider/pprof/tsconfig.json @@ -3,9 +3,9 @@ "compilerOptions": { "rootDir": ".", "outDir": "dist", - "lib": ["ESNext"], + "lib": ["ESNext"] }, "include": ["*.ts"], "exclude": ["dist", "vitest.config.ts"], - "references": [{ "path": "../../lib/provider" }], + "references": [{ "path": "../../lib/provider" }] } From ca751577d98dee6e04e3c126eed044806d2b69e3 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 12 Dec 2024 14:50:46 +0100 Subject: [PATCH 27/29] ci: run biome format --fix --- provider/pprof/index.ts | 5 +++-- provider/pprof/pprof.ts | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/provider/pprof/index.ts b/provider/pprof/index.ts index ac9ea8cd..554b8946 100644 --- a/provider/pprof/index.ts +++ b/provider/pprof/index.ts @@ -107,8 +107,9 @@ const pprof: Provider = { } let item: Item = { - title: `pprof ${top.type}: cum ${node.cum}${top.unit}, ${node.cumPerc}% (#${i + 1 - }, sort=${settings.top?.sort || 'cum'})`, + title: `pprof ${top.type}: cum ${node.cum}${top.unit}, ${node.cumPerc}% (#${ + i + 1 + }, sort=${settings.top?.sort || 'cum'})`, } const list = pprof.list(node.function) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index d728776c..32c86be2 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -30,14 +30,14 @@ export function getPprof(): Pprof | null { if (!stdout.endsWith('not found')) { return new Pprof('go tool pprof') } - } catch (e) { } + } catch (e) {} try { const stdout = execSync('which pprof').toString('utf-8').trim() if (!stdout.endsWith('not found')) { return new Pprof('pprof') } - } catch (e) { } + } catch (e) {} return null } @@ -291,7 +291,7 @@ export class Pprof { } return { - type: reportType ? reportType[1] ?? 'cpu' : '', + type: reportType ? (reportType[1] ?? 'cpu') : '', file: binaryName ? binaryName[1] : undefined, unit: unit ?? 's', nodes: nodes, From f736c665f7ed50c91184e6ea79629d79ede5300d Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Thu, 12 Dec 2024 14:54:02 +0100 Subject: [PATCH 28/29] ci: fix more biome errors --- provider/pprof/pprof.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index 32c86be2..d728776c 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -30,14 +30,14 @@ export function getPprof(): Pprof | null { if (!stdout.endsWith('not found')) { return new Pprof('go tool pprof') } - } catch (e) {} + } catch (e) { } try { const stdout = execSync('which pprof').toString('utf-8').trim() if (!stdout.endsWith('not found')) { return new Pprof('pprof') } - } catch (e) {} + } catch (e) { } return null } @@ -291,7 +291,7 @@ export class Pprof { } return { - type: reportType ? (reportType[1] ?? 'cpu') : '', + type: reportType ? reportType[1] ?? 'cpu' : '', file: binaryName ? binaryName[1] : undefined, unit: unit ?? 's', nodes: nodes, From fa3b4dff4170077fd9caa356926e676782758304 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Sun, 15 Dec 2024 17:18:26 +0100 Subject: [PATCH 29/29] ci: appease biome --- provider/pprof/pprof.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/provider/pprof/pprof.ts b/provider/pprof/pprof.ts index d728776c..5b407e00 100644 --- a/provider/pprof/pprof.ts +++ b/provider/pprof/pprof.ts @@ -30,14 +30,14 @@ export function getPprof(): Pprof | null { if (!stdout.endsWith('not found')) { return new Pprof('go tool pprof') } - } catch (e) { } + } catch (e) {} try { const stdout = execSync('which pprof').toString('utf-8').trim() if (!stdout.endsWith('not found')) { return new Pprof('pprof') } - } catch (e) { } + } catch (e) {} return null }