Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
gregtatum committed Aug 3, 2021
1 parent 000bb7e commit 1f74c79
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
# The profile conversion test includes very verbose snapshots that make code diff
# numbers non-sensical. Jest produces nice diffs if there are mismatches. These
# snapshots behave nicer if the diffs are treated as binary.
src/test/unit/__snapshots__/profile-conversion.test.js.snap binary
src/test/unit/__snapshots__/profile-conversion.test.js.snap -diff
74 changes: 44 additions & 30 deletions src/profile-logic/import/dhat.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
getEmptyThread,
getEmptyUnbalancedNativeAllocationsTable,
} from 'firefox-profiler/profile-logic/data-structures';
import { coerce } from 'firefox-profiler/utils/flow';
import { coerce, ensureExists } from 'firefox-profiler/utils/flow';

/**
* DHAT is a heap memory analysis tool in valgrind. It's also available as rust component.
Expand All @@ -25,7 +25,7 @@ import { coerce } from 'firefox-profiler/utils/flow';
* git clone git://sourceware.org/git/valgrind.git
* dhat/dh_main.c
*/
type DhatJson = {|
type DhatJson = $ReadOnly<{|
// Version number of the format. Incremented on each
// backwards-incompatible change. A mandatory integer.
dhatFileVersion: 2,
Expand Down Expand Up @@ -86,9 +86,9 @@ type DhatJson = {|
// '0x4A9A2BE: _nl_find_locale (findlocale.c:153)'
// ],
ftbl: string[],
|};
|}>;

type ProgramPoint = {|
type ProgramPoint = $ReadOnly<{|
// Total bytes and blocks. Mandatory integers.
tb: Bytes,
tbk: Blocks,
Expand Down Expand Up @@ -132,10 +132,11 @@ type ProgramPoint = {|
// e.g. [5, -3, 4, 2]
acc: number[],

// Frames. Each element is an index into the "ftbl" array below.
// Frames. Each element is an index into the "ftbl" array above.
// The array is ordered from leaf to root.
// - All modes: A mandatory array of integers.
fs: IndexIntoDhatFrames[],
|};
|}>;

// All units of time are in instruction counts.
// Per: https://valgrind.org/docs/manual/dh-manual.html
Expand Down Expand Up @@ -171,7 +172,7 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
profile.meta.product = dhat.cmd + ' (dhat)';
profile.meta.importedFrom = `dhat`;

const allocations = getEmptyUnbalancedNativeAllocationsTable();
const allocationsTable = getEmptyUnbalancedNativeAllocationsTable();
const thread = getEmptyThread();
thread.pid = dhat.pid;
const { funcTable, stringTable, stackTable, frameTable } = thread;
Expand All @@ -190,28 +191,33 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
let line = null;
let column = null;

const result = funcName.match(/^(0x[0-9a-f]+): (.+) \((.+):(\d+):(\d+)\)$/);
// ^(0x[0-9a-f]+): (.+) \((.+):(\d+):(\d+)\)$ Regex
// (1 ) (2 ) (3 ) (4 ) (5 ) Capture groups
const result = funcName.match(
/^0x([0-9a-f]+): (.+) \((.+):(\d+):(\d+)\)$/i
);
// ^0x([0-9a-f]+): (.+) \((.+):(\d+):(\d+)\)$ Regex
// (1 ) (2 ) (3 ) (4 ) (5 ) Capture groups
// ^ $ Start to end
// : \( \) Some raw characters
// (0x[0-9a-f]+) Match the address, e.g. 0x10250148c
// ([0-9a-f]+) Match the address, e.g. 10250148c
// (.+) Match the function name
// (.+) Match the filename
// (\d+) Match the line number
// (\d+) Match the column number

// Example input: "0x10250148c: alloc::vec::Vec<T,A>::append_elements (vec.rs:1469:9)"
// Capture groups: 11111111111 2222222222222222222222222222222222222 333333 4444 5
// Capture groups: 111111111 2222222222222222222222222222222222222 333333 4444 5
if (result) {
address = parseInt(result[1].substr(2), 16);
address = parseInt(result[1], 16);
funcName = result[2];
fileName = result[3];
line = Number(result[4]);
column = Number(result[5]);
}
// If the above regex doesn't match, just use the raw funcName, without additional
// information.

const funcKey = `${funcName} ${fileName}`;

let funcIndex = funcKeyToFuncIndex.get(funcKey);
if (funcIndex === undefined) {
funcTable.name.push(stringTable.indexForString(funcName));
Expand Down Expand Up @@ -242,25 +248,27 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
const endBytes: Bytes[] = [];

for (const pp of dhat.pps) {
// Never reset the stackIndex, as it as stack indexes always growing taller.
// Never reset the stackIndex, stack indexes always growing larger.
let stackIndex = -1;
let prefix = null;

// Go from root to tip on the backtrace.
for (let i = pp.fs.length - 1; i >= 0; i--) {
// The dhat frame indexes matches the process profile frame index.
const frameIndex = pp.fs[i];
const funcIndex = frameTable.func[frameIndex];
const funcIndex = ensureExists(
frameTable.func[frameIndex],
'Expected to find a funcIndex from a frameIndex'
);

// Case 1: The stack index starts at -1, increment by 1 to start searching stacks
// at index 0.
// Case 2: This is the previously matched stack index, increment it by 1 to continue
// searching at the next stack index.
stackIndex++;

// Start searching for a stack index.
for (
// Increase the stackIndex by one. For the first run, this will end up
// being a stackIndex of 0. For the consecutive runs, it will be the next
// stack index.
stackIndex++;
stackIndex < stackTable.length;
stackIndex++
) {
for (; stackIndex < stackTable.length; stackIndex++) {
const nextFrameIndex = stackTable.frame[stackIndex];
if (
frameTable.func[nextFrameIndex] === funcIndex &&
Expand Down Expand Up @@ -290,23 +298,29 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
bytesAtGmax.push(pp.gb);
endBytes.push(pp.eb);

allocations.time.push(0);
allocations.stack.push(stackIndex);
allocations.length++;
allocationsTable.time.push(0);
allocationsTable.stack.push(stackIndex);
allocationsTable.length++;
}

const totalBytesThread = { ...thread };
const maximumBytesThread = { ...thread };
const bytesAtGmaxThread = { ...thread };
const endBytesThread = { ...thread };

totalBytesThread.nativeAllocations = { ...allocations, weight: totalBytes };
totalBytesThread.nativeAllocations = {
...allocationsTable,
weight: totalBytes,
};
maximumBytesThread.nativeAllocations = {
...allocations,
...allocationsTable,
weight: maximumBytes,
};
bytesAtGmaxThread.nativeAllocations = { ...allocations, weight: bytesAtGmax };
endBytesThread.nativeAllocations = { ...allocations, weight: endBytes };
bytesAtGmaxThread.nativeAllocations = {
...allocationsTable,
weight: bytesAtGmax,
};
endBytesThread.nativeAllocations = { ...allocationsTable, weight: endBytes };

totalBytesThread.name = 'Total Bytes';
maximumBytesThread.name = 'Maximum Bytes';
Expand Down

0 comments on commit 1f74c79

Please sign in to comment.