Skip to content

DHAT: Optimize import time for large profiles #4640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
julienw merged 3 commits into from
Jun 6, 2023
Merged

DHAT: Optimize import time for large profiles #4640

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d873aa
feat: optimize DHAT import tree building algorithm
flowln May 28, 2023
bed5b29
refactor(DHAT): improve code readability of the stack tree builder
flowln May 30, 2023
9726cce
Merge branch 'main' into optimize_dhat_import
julienw Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions src/profile-logic/import/dhat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
Pid,
Bytes,
IndexIntoFuncTable,
IndexIntoStackTable,
} from 'firefox-profiler/types';

import {
Expand Down Expand Up @@ -253,9 +254,12 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
const bytesAtGmax: Bytes[] = [];
const endBytes: Bytes[] = [];

// Maps prefixes to their descendents (more specific prefixes).
const postfix: Map<IndexIntoStackTable | null, IndexIntoStackTable[]> =
new Map();

for (const pp of dhat.pps) {
// Never reset the stackIndex, stack indexes always growing larger.
let stackIndex = -1;
let stackIndex = 0;
Comment on lines -258 to +262

@julienw julienw May 30, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this value isn't used outside of the loop now, so it can be moved below. Also look at the other comment about the same stackIndex variable below.

@flowln flowln May 30, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable is still used one time after the loop, to tell the ending stackIndex of the PP entry to the allocationsTable!

let prefix = null;

// Go from root to tip on the backtrace.
Expand All @@ -267,20 +271,21 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
'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 (; stackIndex < stackTable.length; stackIndex++) {
const nextFrameIndex = stackTable.frame[stackIndex];
if (
frameTable.func[nextFrameIndex] === funcIndex &&
stackTable.prefix[stackIndex] === prefix
) {
break;
// We want this to be the fallback, so that a stack index gets created when the 'if' below fails,
// or when we don't find a matching frame inside that loop.
stackIndex = stackTable.length;

// List of possible stack indexes to look for.
const candidateStackTables = postfix.get(prefix);
if (candidateStackTables) {
// Start searching for a stack index.
for (const sliceStackIndex of candidateStackTables) {
const nextFrameIndex = stackTable.frame[sliceStackIndex];
// No need to look for the prefix, since candidateStackTables already takes that into account.
if (frameTable.func[nextFrameIndex] === funcIndex) {
stackIndex = sliceStackIndex;
break;
}
}
}

Expand All @@ -290,6 +295,15 @@ export function attemptToConvertDhat(json: mixed): Profile | null {
stackTable.category.push(otherCategory);
stackTable.category.push(otherSubCategory);
stackTable.prefix.push(prefix);

if (candidateStackTables) {
// Append us to the list of possible stack indexes of our parent.
candidateStackTables.push(stackIndex);
} else {
// We are the first descendents of our parent.
postfix.set(prefix, [stackIndex]);
}

// The stack index already points to this spot.
stackTable.length++;
}
Expand Down