Skip to content

Commit

Permalink
import: callgrind.ts: Improved collection of file names associated wi…
Browse files Browse the repository at this point in the history
…th 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]>
  • Loading branch information
glatosinski committed Feb 29, 2024
1 parent 0121cf9 commit 20072c5
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions src/import/callgrind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ class CallgrindParser {

private filename: string | null = null
private functionName: string | null = null
private functionFile: string | null = null
private currentFunctionFile: string | null = null
private calleeFilename: string | null = null
private calleeFunctionName: string | null = null

Expand Down Expand Up @@ -396,7 +398,7 @@ class CallgrindParser {
}

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

case 'fl': {
this.filename = this.parseNameWithCompression(value, this.savedFileNames)
// The 'fl' needs to be stored in order to reset current filename upon
// consecutive 'fn' calls
this.currentFunctionFile = this.filename
break
}

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

Check failure on line 472 in src/import/callgrind.ts

View workflow job for this annotation

GitHub Actions / test (18.x)

Delete `·`

Check failure on line 472 in src/import/callgrind.ts

View workflow job for this annotation

GitHub Actions / test (20.x)

Delete `·`
this.functionFile = this.filename
break
}

Expand Down

0 comments on commit 20072c5

Please sign in to comment.