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

feat(profiling): add text renderer #31108

Merged
merged 4 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
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
5 changes: 5 additions & 0 deletions static/app/utils/profiling/flamegraph/FlamegraphTheme.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const MONOSPACE_FONT = `ui-monospace, Menlo, Monaco, 'Cascadia Mono', 'Segoe UI
'Oxygen Mono', 'Ubuntu Monospace', 'Source Code Pro', 'Fira Mono', 'Droid Sans Mono',
'Courier New', monospace`;

const FRAME_FONT = `"Source Code Pro", Courier, monospace`;

// Luma chroma hue settings
export interface LCH {
C_0: number;
Expand Down Expand Up @@ -85,6 +87,7 @@ export interface FlamegraphTheme {
};
FONTS: {
FONT: string;
FRAME_FONT: string;
};
}

Expand Down Expand Up @@ -155,6 +158,7 @@ export const LightFlamegraphTheme: FlamegraphTheme = {
},
FONTS: {
FONT: MONOSPACE_FONT,
FRAME_FONT,
},
};

Expand Down Expand Up @@ -210,5 +214,6 @@ export const DarkFlamegraphTheme: FlamegraphTheme = {
},
FONTS: {
FONT: MONOSPACE_FONT,
FRAME_FONT,
},
};
44 changes: 44 additions & 0 deletions static/app/utils/profiling/gl/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,3 +383,47 @@ function getContext(canvas: HTMLCanvasElement, context: string): RenderingContex
}

export {getContext};

/** Find closest min and max value to target */
export function findRangeBinarySearch(
{low, high}: {low: number; high: number},
fn: (val: number) => number,
target: number,
precision = 1
): [number, number] {
if (target < low || target > high) {
throw new Error(
`Target value needs to be in low-high range, got ${target} for [${low}, ${high}]`
);
}
// eslint-disable-next-line
while (true) {
if (high - low <= precision) {
return [low, high];
}

const mid = (high + low) / 2;
if (fn(mid) < target) {
low = mid;
} else {
high = mid;
}
}
}

export const ELLIPSIS = '\u2026';
export function trimTextCenter(text: string, low: number) {
if (low > text.length) {
return text;
}

const prefixLength = Math.floor(low / 2);
// Use 1 character less than the low value to account for ellipsis
// and favor displaying the prefix
const postfixLength = low - prefixLength - 1;

return `${text.substring(0, prefixLength)}${ELLIPSIS}${text.substring(
text.length - postfixLength,
text.length
)}`;
}
149 changes: 149 additions & 0 deletions static/app/utils/profiling/renderers/textRenderer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import {mat3} from 'gl-matrix';

import {Flamegraph} from '../flamegraph';
import {FlamegraphTheme} from '../flamegraph/FlamegraphTheme';
import {FlamegraphFrame} from '../flamegraphFrame';
import {
ELLIPSIS,
findRangeBinarySearch,
getContext,
Rect,
resizeCanvasToDisplaySize,
trimTextCenter,
} from '../gl/utils';

export function isOutsideView(frame: Rect, view: Rect, inverted: boolean): boolean {
// Frame is outside of the view on the left
if (!frame.overlaps(view)) {
return true;
}

// @TODO check if we still need this
if (inverted) {
Copy link
Member

Choose a reason for hiding this comment

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

What does inverted mean here?

Copy link
Member Author

Choose a reason for hiding this comment

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

inverted aka top-down flamegraph

if (frame.top - 1 >= view.bottom) {
return true;
}
}

return false;
}

class TextRenderer {
textCache: Record<string, number> = {};

canvas: HTMLCanvasElement;
context: CanvasRenderingContext2D;
theme: FlamegraphTheme;
flamegraph: Flamegraph;

constructor(canvas: HTMLCanvasElement, flamegraph: Flamegraph, theme: FlamegraphTheme) {
this.canvas = canvas;
this.theme = theme;
this.flamegraph = flamegraph;

this.context = getContext(canvas, '2d');

resizeCanvasToDisplaySize(canvas);
}

clearCache(): void {
this.textCache = {};
}

measureText(context: CanvasRenderingContext2D, text: string): number {
if (this.textCache[text]) {
return this.textCache[text];
}
this.textCache[text] = context.measureText(text).width;
return this.textCache[text];
}

draw(configViewSpace: Rect, configSpace: Rect, configToPhysicalSpace: mat3): void {
this.context.font = `${this.theme.SIZES.BAR_FONT_SIZE * window.devicePixelRatio}px ${
this.theme.FONTS.FRAME_FONT
}`;

this.context.textBaseline = 'alphabetic';
this.context.fillStyle = this.theme.COLORS.LABEL_FONT_COLOR;

const minWidth = this.measureText(this.context, ELLIPSIS);

let frame: FlamegraphFrame;
let i: number;
const length: number = this.flamegraph.frames.length;

// We currently iterate over all frames, but we could optimize this to only iterate over visible frames.
// This could be achieved by querying the flamegraph tree (as an interval tree). This would still run in O(n), but
// would improve our best case performance (e.g. when users) zoom into the flamegraph
for (i = 0; i < length; i++) {
frame = this.flamegraph.frames[i];
Zylphrex marked this conversation as resolved.
Show resolved Hide resolved

// This rect gets discarded after each render which is wasteful
const frameInConfigSpace = new Rect(
frame.start,
this.flamegraph.inverted ? configSpace.height - frame.depth + 1 : frame.depth,
frame.end - frame.start,
1
);

// Check if our rect overlaps with the current viewport and skip it
if (
isOutsideView(frameInConfigSpace, configViewSpace, !!this.flamegraph.inverted)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is because I'm not sure what inverted means, but if inverted is already used to initialize the Rect, it shouldn't be needed for this isOutsideView check right?

Copy link
Member Author

@JonasBa JonasBa Jan 20, 2022

Choose a reason for hiding this comment

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

Yes, we dont initialize inverted with Rect right now because I dont want to tie it to the flamegraph. The problem is that currently we kind of fake invert rectangles... we remap their depth, whereas the real solution would be to change coordinate system. That could be a good task if you want to take a stab at it. It would allow us to simplify some of the code and make it more straightforward

) {
continue;
}

// We pin the start and end of the frame, so scrolling around keeps text pinned to the left or right side of the viewport
const pinnedStart = Math.max(frame.start, configViewSpace.left);
const pinnedEnd = Math.min(frame.end, configViewSpace.right);

// This rect gets discarded after each render which is wasteful
const offsetFrame = new Rect(
pinnedStart,
frameInConfigSpace.y,
pinnedEnd - pinnedStart,
1
);

// Transform frame to physical space coordinates
const frameInPhysicalSpace = offsetFrame.transformRect(configToPhysicalSpace);

// Since the text is not exactly aligned to the left/right bounds of the frame, we need to subtract the padding
// from the total width, so that we can truncate the center of the text accurately.
const paddedRectangleWidth =
frameInPhysicalSpace.width -
2 * (this.theme.SIZES.BAR_PADDING * window.devicePixelRatio);

// We want to draw the text in the vertical center of the frame, so we substract half the height of the text
const y =
frameInPhysicalSpace.y -
(this.theme.SIZES.BAR_FONT_SIZE / 2) * window.devicePixelRatio;

// Offset x by 1x the padding
const x =
frameInPhysicalSpace.x + this.theme.SIZES.BAR_PADDING * window.devicePixelRatio;

// If the width of the text is greater than the minimum width to render, we should render it
if (paddedRectangleWidth >= minWidth) {
let text = frame.frame.name;
const textWidth = this.measureText(this.context, text);

// If text width is smaller than rectangle, just draw the text
if (textWidth > paddedRectangleWidth) {
text = trimTextCenter(
text,
findRangeBinarySearch(
{low: 0, high: text.length},
n => this.measureText(this.context, text.substring(0, n)),
paddedRectangleWidth
)[0]
);
}

this.context.fillText(text, x, y);
}
}
}
}

export {TextRenderer};
64 changes: 64 additions & 0 deletions tests/js/spec/utils/profiling/gl/utils.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ import {mat3, vec2} from 'gl-matrix';
import {
createProgram,
createShader,
ELLIPSIS,
findRangeBinarySearch,
getContext,
makeProjectionMatrix,
Rect,
Transform,
trimTextCenter,
} from 'sentry/utils/profiling/gl/utils';

describe('makeProjectionMatrix', () => {
Expand Down Expand Up @@ -304,3 +307,64 @@ describe('Rect', () => {
});
});
});

describe('findRangeBinarySearch', () => {
it('throws if target is out of range', () => {
expect(() =>
findRangeBinarySearch({low: 1, high: 2}, () => 0, 0, Number.MIN_SAFE_INTEGER)
).toThrow('Target value needs to be in low-high range, got 0 for [1, 2]');
});

it('finds in single iteration', () => {
const text = new Array(10)
.fill(0)
.map((_, i) => String.fromCharCode(i + 97))
.join('');

const fn = jest.fn().mockImplementation(n => {
return text.substring(0, n).length;
});

const target = 2;
const precision = 1;

// First iteration will halve 1+3, next iteration will compare 2-1 <= 1 and return [1,2]
const [low, high] = findRangeBinarySearch({low: 1, high: 3}, fn, target, precision);

expect([low, high]).toEqual([1, 2]);
expect(fn).toHaveBeenCalledTimes(1);
expect(text.substring(0, low)).toBe('a');
});

it('finds closest range', () => {
const text = new Array(10)
.fill(0)
.map((_, i) => String.fromCharCode(i + 97))
.join('');

const fn = jest.fn().mockImplementation(n => {
return text.substring(0, n).length;
});

const target = 4;
const precision = 1;

const [low, high] = findRangeBinarySearch({low: 0, high: 10}, fn, target, precision);

expect([low, high]).toEqual([3.75, 4.375]);
expect(fn).toHaveBeenCalledTimes(4);
expect(text.substring(0, low)).toBe('abc');
});
});

describe('trimTextCenter', () => {
it('trims nothing if low > length', () => {
expect(trimTextCenter('abc', 4)).toBe('abc');
});
it('trims center perfectly', () => {
expect(trimTextCenter('abcdef', 5)).toBe(`ab${ELLIPSIS}ef`);
});
it('favors prefix length', () => {
expect(trimTextCenter('abcdef', 4)).toBe(`ab${ELLIPSIS}f`);
});
});
Loading