Skip to content
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.

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

Speed up Critical path calculation #1975

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,64 +22,49 @@ import sanitizeOverFlowingChildren from './utils/sanitizeOverFlowingChildren';
* Computes the critical path sections of a Jaeger trace.
* The algorithm begins with the top-level span and iterates through the last finishing children (LFCs).
* It recursively computes the critical path for each LFC span.
* Upon return from recursion, the algorithm walks backward and picks another child that
* finished just before the LFC's start.
* @param spanMap - A map associating span IDs with spans.
* @param spanId - The ID of the current span.
* @param criticalPath - An array of critical path sections.
* @param returningChildStartTime - Optional parameter representing the span's start time.
* It is provided only during the recursive return phase.
* @returns - An array of critical path sections for the trace.
* @example -
* |-------------spanA--------------|
* |--spanB--| |--spanC--|
* The LFC of spanA is spanC, as it finishes last among its child spans.
* After invoking CP recursively on LFC, for spanC there is no LFC, so the algorithm walks backward.
* At this point, it uses returningChildStartTime (startTime of spanC) to select another child that finished
* immediately before the LFC's start.
*/
const computeCriticalPath = (
spanMap: Map<string, Span>,
spanId: string,
criticalPath: criticalPathSection[],
returningChildStartTime?: number
criticalPath: criticalPathSection[]
): criticalPathSection[] => {
const currentSpan: Span = spanMap.get(spanId)!;
let criticalPathBoundary = currentSpan.startTime + currentSpan.duration;
Copy link
Member

Choose a reason for hiding this comment

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

confusing name, boundary can be either top or bottom


const lastFinishingChildSpan = findLastFinishingChildSpan(spanMap, currentSpan, returningChildStartTime);
let spanCriticalSection: criticalPathSection;

if (lastFinishingChildSpan) {
spanCriticalSection = {
spanId: currentSpan.spanID,
section_start: lastFinishingChildSpan.startTime + lastFinishingChildSpan.duration,
section_end: returningChildStartTime || currentSpan.startTime + currentSpan.duration,
};
if (spanCriticalSection.section_start !== spanCriticalSection.section_end) {
criticalPath.push(spanCriticalSection);
}
// Now focus shifts to the lastFinishingChildSpan of cuurent span
computeCriticalPath(spanMap, lastFinishingChildSpan.spanID, criticalPath);
} else {
// If there is no last finishing child then total section upto startTime of span is on critical path
spanCriticalSection = {
spanId: currentSpan.spanID,
section_start: currentSpan.startTime,
section_end: returningChildStartTime || currentSpan.startTime + currentSpan.duration,
};
if (spanCriticalSection.section_start !== spanCriticalSection.section_end) {
criticalPath.push(spanCriticalSection);
}
// Now as there are no lfc's focus shifts to parent span from startTime of span
// return from recursion and walk backwards to one level depth to parent span
// provide span's startTime as returningChildStartTime
if (currentSpan.references.length) {
const parentSpanId: string = currentSpan.references.filter(
reference => reference.refType === 'CHILD_OF'
)[0].spanID;
computeCriticalPath(spanMap, parentSpanId, criticalPath, currentSpan.startTime);
for (let i = 0; i < currentSpan.childSpanIds.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

is there an assumption on the order of this array?

const child: Span = spanMap.get(currentSpan.childSpanIds[i])!;
const childEndTime = child.startTime + child.duration;
if (childEndTime <= criticalPathBoundary) {
const spanCriticalSection = {
spanId: currentSpan.spanID,
section_start: childEndTime,
section_end: criticalPathBoundary,
};
Copy link
Member

Choose a reason for hiding this comment

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

I don't follow. criticalPathBoundary is initially end-of-parent. This says if the child ends before than, then the difference between child.end and parent.end is the critical path contributed by that span?

if (spanCriticalSection.section_start < spanCriticalSection.section_end) {
criticalPath.push(spanCriticalSection);
}
computeCriticalPath(spanMap, child.spanID, criticalPath);
criticalPathBoundary = child.startTime;
}
}

const spanCriticalSection = {
spanId: currentSpan.spanID,
section_start: currentSpan.startTime,
section_end: criticalPathBoundary,
};
if (spanCriticalSection.section_start < spanCriticalSection.section_end) {
criticalPath.push(spanCriticalSection);
}

return criticalPath;
};

Expand Down