Skip to content

Commit 60be11b

Browse files
committed
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<string, Func>
1 parent af0cb32 commit 60be11b

File tree

4 files changed

+42
-47
lines changed

4 files changed

+42
-47
lines changed

provider/pprof/index.ts

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -61,36 +61,32 @@ const pprof: Provider = {
6161
}
6262

6363
const anns: Annotation[] = []
64+
top.nodes.forEach((node: Node, i: number) => {
65+
const func = content.funcs[node.function]
66+
if (!func) {
67+
return
68+
}
6469

65-
// TODO(1): turn Func[] into a Record<string, Func> for faster lookups (top output should be ordered, funcs don't need to).
66-
for (const func of content.funcs) {
67-
top.nodes.forEach((node: Node, i: number) => {
68-
if (func.pprofRegex !== node.function) {
69-
return
70-
}
71-
72-
let item: Item = {
73-
title: `[#${i + 1}] ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`,
74-
}
70+
let item: Item = {
71+
title: `[#${i + 1}] ${top.type}: ${node.cum}${top.unit}, ${node.cumPerc}% (cum)`,
72+
}
7573

76-
const list = pprof.list(node.function)
77-
if (list) {
78-
item = {
79-
...item,
80-
ai: {
81-
content: "Output of 'pprof -list' command for this function:\n" + list.raw,
82-
},
83-
}
74+
const list = pprof.list(node.function)
75+
if (list) {
76+
item = {
77+
...item,
78+
ai: {
79+
content: "Output of 'pprof -list' command for this function:\n" + list.raw,
80+
},
8481
}
82+
}
8583

86-
anns.push({
87-
uri: params.uri,
88-
range: func.range,
89-
item: item,
90-
})
84+
anns.push({
85+
uri: params.uri,
86+
range: func.range,
87+
item: item,
9188
})
92-
}
93-
89+
})
9490
return anns
9591
},
9692
}

provider/pprof/parser.test.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,35 +21,30 @@ func (t Thing) String() string { return "thing" }
2121

2222
expect(parseGolang(content)).toStrictEqual<Contents>({
2323
package: 'example',
24-
funcs: [
25-
{
24+
funcs: {
25+
'example.A': {
2626
name: 'A',
2727
range: { start: { line: 4, character: 5 }, end: { line: 4, character: 6 } },
28-
pprofRegex: 'example.A',
2928
},
30-
{
29+
'example.b_func': {
3130
name: 'b_func',
3231
range: { start: { line: 5, character: 5 }, end: { line: 5, character: 11 } },
33-
pprofRegex: 'example.b_func',
3432
},
35-
{
33+
'example.A2': {
3634
name: 'A2',
3735
range: { start: { line: 6, character: 5 }, end: { line: 6, character: 7 } },
38-
pprofRegex: 'example.A2',
3936
},
40-
{
37+
'example.(*Thing).doStuff': {
4138
name: 'doStuff',
4239
range: { start: { line: 12, character: 16 }, end: { line: 12, character: 23 } },
43-
pprofRegex: 'example.(*Thing).doStuff',
4440
receiver: '*Thing',
4541
},
46-
{
42+
'example.(Thing).String': {
4743
name: 'String',
4844
range: { start: { line: 13, character: 15 }, end: { line: 13, character: 21 } },
49-
pprofRegex: 'example.(Thing).String',
5045
receiver: 'Thing',
5146
},
52-
],
47+
},
5348
})
5449
})
5550
})

provider/pprof/parser.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@ const methodRegex = /^func \(\w+ (\*)?(\w+)\) (\w+)(?:\()/m
66

77
export interface Contents {
88
package: string
9-
funcs: Func[]
9+
10+
/** The key for each function is its fully-qualified name, e.g. example.MyFunc or example.(*Thing).MyMethod,
11+
* as they are unique within the file.
12+
*/
13+
funcs: Record<string, Func>
1014
}
1115

1216
/** Func is a Go function or method with additional metadata to help locate it in the file and filter for in in `pprof`. */
1317
export interface Func {
1418
name: string
1519
range: Range
16-
pprofRegex: string
1720
receiver?: string
1821
}
1922

@@ -25,7 +28,7 @@ export function parseGolang(source: string): Contents | null {
2528
const pkg = pkgMatch[1]
2629
const result: Contents = {
2730
package: pkg,
28-
funcs: [],
31+
funcs: {},
2932
}
3033

3134
const lines = source.split('\n')
@@ -51,14 +54,13 @@ export function parseGolang(source: string): Contents | null {
5154
const start = 5 // "func ".length
5255
const { func, end } = readFuncName(start)
5356

54-
result.funcs.push({
57+
result.funcs[`${pkg}.${func}`] = {
5558
name: func,
5659
range: {
5760
start: { line: i, character: start },
5861
end: { line: i, character: end },
5962
},
60-
pprofRegex: `${pkg}.${func}`,
61-
})
63+
}
6264
break
6365
}
6466
case methodRegex.test(line): {
@@ -81,15 +83,14 @@ export function parseGolang(source: string): Contents | null {
8183
const start = rparen + 2
8284
const { func, end } = readFuncName(start)
8385

84-
result.funcs.push({
86+
result.funcs[`${pkg}.(${receiver}).${func}`] = {
8587
name: func,
8688
range: {
8789
start: { line: i, character: start },
8890
end: { line: i, character: end },
8991
},
90-
pprofRegex: `${pkg}.(${receiver}).${func}`,
9192
receiver: receiver,
92-
})
93+
}
9394
break
9495
}
9596
}

provider/pprof/pprof.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ export class Pprof {
202202
this.sources = sources
203203
}
204204

205-
// TODO: return raw output
206205
top(options: TopOptions): TopOutput | null {
207206
if (!this.sources.report) {
208207
return null
@@ -271,6 +270,10 @@ export class Pprof {
271270
unit = node.unit
272271
}
273272

273+
// Should we include the raw output here the way we do in list()?
274+
// It may include entries for other functions in the same package
275+
// which are declared in different files, which might be a useful
276+
// information to Cody.
274277
nodes.push({
275278
function: node.func,
276279
flat: parseFloat(node.flat),

0 commit comments

Comments
 (0)