Skip to content

Commit 20072c5

Browse files
committed
import: callgrind.ts: Improved collection of file names associated with functions
When called function is within an inlined code block, it should use file names provided by 'fi' or 'fe' commands. Also, when 'fn' command is called, it sets the current file name to the value provided in the previous 'fl' call. Also, the file associated with 'fn' should be stored separately so that it is not overwritten by new 'fl' calls (Callgrind does not tell whether there can be multiple 'fl' calls without following 'fn' calls, and this should guard the parser from associating the wrong new file name with the previous 'fn' function). This modification fixes detached call stacks, which were created when one function has been called from inline block without the file source update, which led to duplicated entries, which were later not removed when determining root functions. Signed-off-by: Grzegorz Latosinski <[email protected]>
1 parent 0121cf9 commit 20072c5

File tree

1 file changed

+17
-8
lines changed

1 file changed

+17
-8
lines changed

src/import/callgrind.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,8 @@ class CallgrindParser {
342342

343343
private filename: string | null = null
344344
private functionName: string | null = null
345+
private functionFile: string | null = null
346+
private currentFunctionFile: string | null = null
345347
private calleeFilename: string | null = null
346348
private calleeFunctionName: string | null = null
347349

@@ -396,7 +398,7 @@ class CallgrindParser {
396398
}
397399

398400
private frameInfo(): FrameInfo {
399-
const file = this.filename || '(unknown)'
401+
const file = this.functionFile || '(unknown)'
400402
const name = this.functionName || '(unknown)'
401403
const key = `${file}:${name}`
402404
return {key, name, file}
@@ -445,23 +447,30 @@ class CallgrindParser {
445447
case 'fe':
446448
case 'fi': {
447449
// fe/fi are used to indicate the source-file of a function definition
448-
// changed mid-definition. This is for inlined code, but doesn't
449-
// indicate that we've actually switched to referring to a different
450-
// function, so we mostly ignore it.
451-
//
452-
// We still need to do the parseNameWithCompression call in case a name
453-
// is defined and then referenced later on for name compression.
454-
this.parseNameWithCompression(value, this.savedFileNames)
450+
// changed mid-definition. This is for inlined code, but when a function
451+
// is called from within the inlined code, it is indicated that it
452+
// comes from file marked by fe/fi.
453+
this.filename = this.parseNameWithCompression(value, this.savedFileNames)
455454
break
456455
}
457456

458457
case 'fl': {
459458
this.filename = this.parseNameWithCompression(value, this.savedFileNames)
459+
// The 'fl' needs to be stored in order to reset current filename upon
460+
// consecutive 'fn' calls
461+
this.currentFunctionFile = this.filename
460462
break
461463
}
462464

463465
case 'fn': {
466+
// the file for a function defined with 'fn' always comes from 'fl'
467+
// It also resets the current filename to previous 'fl' value
468+
// https://github.com/KDE/kcachegrind/blob/ea4314db2785cb8f279fe884ee7f82445642b692/libcore/cachegrindloader.cpp#L808
469+
if (this.filename !== this.currentFunctionFile) this.filename = this.currentFunctionFile
464470
this.functionName = this.parseNameWithCompression(value, this.savedFunctionNames)
471+
// since the format of callgrind does not disallow to define multiple 'fl' without following 'fn',
472+
// it is important to store the associated filename for the current function for frameInfo method
473+
this.functionFile = this.filename
465474
break
466475
}
467476

0 commit comments

Comments
 (0)