Skip to content

Commit

Permalink
Fix performance issues for the caller/callee flamegraphs in the sandw…
Browse files Browse the repository at this point in the history
…ich view (#296)

This fixes two unrelated problems which together caused performance issues in the sandwich view & made hover tooltips appear to be broken.

The first issue was caused by continuously priming the `requestAnimationFrame` loop when it should be a no-op, and the second issue was caused by using different cache keys when trying to access a memoized value in the caller & callee flamegraph components. This resulted in thrash, and especially bad performance because the cache miss was resulting in us re-allocating the WebGL framebuffer on every frame, which is unsurprisingly quite slow.

Fixes #212 
Fixes #155 
Fixes #74 (though this was maybe already fixed)
  • Loading branch information
jlfwong authored Jul 19, 2020
1 parent ff447c2 commit 7514f4c
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 11 deletions.
4 changes: 0 additions & 4 deletions src/store/getters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ export const getRowAtlas = memoizeByReference((canvasContext: CanvasContext) =>
)
})

export const getProfileWithRecursionFlattened = memoizeByReference((profile: Profile) =>
profile.getProfileWithRecursionFlattened(),
)

export const getProfileToView = memoizeByShallowEquality(
({profile, flattenRecursion}: {profile: Profile; flattenRecursion: boolean}): Profile => {
return flattenRecursion ? profile.getProfileWithRecursionFlattened() : profile
Expand Down
8 changes: 6 additions & 2 deletions src/views/flamechart-pan-zoom-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export interface FlamechartPanZoomViewProps {
setConfigSpaceViewportRect: (rect: Rect) => void

logicalSpaceViewportSize: Vec2
setLogicalSpaceViewportBounds: (size: Vec2) => void
setLogicalSpaceViewportSize: (size: Vec2) => void
}

export class FlamechartPanZoomView extends Component<FlamechartPanZoomViewProps, {}> {
Expand Down Expand Up @@ -396,7 +396,11 @@ export class FlamechartPanZoomView extends Component<FlamechartPanZoomViewProps,
),
)
}
this.props.setLogicalSpaceViewportBounds(new Vec2(width, height))

const newSize = new Vec2(width, height)
if (!newSize.equals(logicalSpaceViewportSize)) {
this.props.setLogicalSpaceViewportSize(newSize)
}
}

onWindowResize = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/views/flamechart-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class FlamechartView extends StatelessComponent<FlamechartViewProps> {
configSpaceViewportRect={this.props.configSpaceViewportRect}
setConfigSpaceViewportRect={this.setConfigSpaceViewportRect}
logicalSpaceViewportSize={this.props.logicalSpaceViewportSize}
setLogicalSpaceViewportBounds={this.setLogicalSpaceViewportSize}
setLogicalSpaceViewportSize={this.setLogicalSpaceViewportSize}
/>
{this.renderTooltip()}
{this.props.selectedNode && (
Expand Down
2 changes: 1 addition & 1 deletion src/views/flamechart-wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class FlamechartWrapper extends StatelessComponent<FlamechartViewProps> {
canvasContext={this.props.canvasContext}
renderInverted={this.props.renderInverted}
logicalSpaceViewportSize={this.props.logicalSpaceViewportSize}
setLogicalSpaceViewportBounds={this.setLogicalSpaceViewportSize}
setLogicalSpaceViewportSize={this.setLogicalSpaceViewportSize}
/>
{this.renderTooltip()}
</div>
Expand Down
3 changes: 0 additions & 3 deletions src/views/inverted-caller-flamegraph-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
getCanvasContext,
createGetColorBucketForFrame,
createGetCSSColorForFrame,
getProfileWithRecursionFlattened,
getFrameToColorBucket,
} from '../store/getters'
import {FlamechartID} from '../store/flamechart-view-state'
Expand Down Expand Up @@ -65,8 +64,6 @@ export const InvertedCallerFlamegraphView = memo((ownProps: FlamechartViewContai
if (!callerCallee) throw new Error('callerCallee missing')
const {selectedFrame} = callerCallee

profile = flattenRecursion ? getProfileWithRecursionFlattened(profile) : profile

const frameToColorBucket = getFrameToColorBucket(profile)
const getColorBucketForFrame = createGetColorBucketForFrame(frameToColorBucket)
const getCSSColorForFrame = createGetCSSColorForFrame(frameToColorBucket)
Expand Down

0 comments on commit 7514f4c

Please sign in to comment.