From 767532fb0f03b26c83f5aaa162084de3d86a5a6f Mon Sep 17 00:00:00 2001 From: Mike Egorov Date: Mon, 28 Jun 2021 17:54:42 +0300 Subject: [PATCH 1/5] improve toooltip rendering --- .../components/FlameGraphRenderer.jsx | 58 ++++++++----------- webapp/sass/flamebearer.scss | 10 +++- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/webapp/javascript/components/FlameGraphRenderer.jsx b/webapp/javascript/components/FlameGraphRenderer.jsx index d2187d494b..78a61e4d78 100644 --- a/webapp/javascript/components/FlameGraphRenderer.jsx +++ b/webapp/javascript/components/FlameGraphRenderer.jsx @@ -308,6 +308,7 @@ class FlameGraphRenderer extends React.Component { this.graphWidth / numTicks / (this.rangeMax - this.rangeMin); this.canvas.height = PX_PER_LEVEL * (levels.length - this.topLevel); this.canvas.style.height = `${this.canvas.height}px`; + this.canvas.style.cursor = "pointer"; if (devicePixelRatio > 1) { this.canvas.width *= 2; @@ -319,7 +320,7 @@ class FlameGraphRenderer extends React.Component { this.ctx.font = '400 12px system-ui, -apple-system, "Segoe UI", "Roboto", "Ubuntu", "Cantarell", "Noto Sans", sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji"'; - const formatter = this.createFormatter(); + this.formatter = this.createFormatter(); // i = level for (let i = 0; i < levels.length - this.topLevel; i++) { const level = levels[this.topLevel + i]; @@ -388,7 +389,7 @@ class FlameGraphRenderer extends React.Component { if (!collapsed && sw >= LABEL_THRESHOLD) { const percent = formatPercent(ratio); - const name = `${names[level[j + 3]]} (${percent}, ${formatter.format(numBarTicks, sampleRate)})`; + const name = `${names[level[j + 3]]} (${percent}, ${this.formatter.format(numBarTicks, sampleRate)})`; this.ctx.save(); this.ctx.clip(); @@ -412,8 +413,6 @@ class FlameGraphRenderer extends React.Component { return; } - this.canvas.style.cursor = "pointer"; - const level = this.state.levels[i]; const x = Math.max(this.tickToX(level[j]), 0); const y = (i - this.topLevel) * PX_PER_LEVEL; @@ -428,44 +427,35 @@ class FlameGraphRenderer extends React.Component { // a little hacky but this is here so that we can get tooltipWidth after text is updated. const tooltipTitle = this.state.names[level[j + 3]]; - tooltipEl.children[0].innerText = tooltipTitle; - const tooltipWidth = tooltipEl.clientWidth; - const formatter = this.createFormatter(); - - this.setState({ - highlightStyle: { - display: "block", - left: `${this.canvas.offsetLeft + x}px`, - top: `${this.canvas.offsetTop + y}px`, - width: `${sw}px`, - height: `${PX_PER_LEVEL}px`, - }, - tooltipStyle: { - display: "block", - left: `${ - Math.min( - this.canvas.offsetLeft + e.nativeEvent.offsetX + 15 + tooltipWidth, - this.canvas.offsetLeft + this.graphWidth - ) - tooltipWidth - }px`, - top: `${this.canvas.offsetTop + e.nativeEvent.offsetY + 12}px`, - }, - tooltipTitle, - tooltipSubtitle: `${percent}, ${numberWithCommas( - numBarTicks - )} samples, ${formatter.format(numBarTicks, this.state.sampleRate)}`, - }); + this.setState( + { + highlightStyle: { + opacity: 1, + transform: `translate(${this.canvas.offsetLeft + x}px, ${this.canvas.offsetTop + y}px)`, + width: `${sw}px`, + height: `${PX_PER_LEVEL}px`, + }, + tooltipStyle: { + opacity: 1, + transform: `translate(${e.clientX+12}px, ${e.clientY+12}px)`, + }, + tooltipTitle, + tooltipSubtitle: `${percent}, ${numberWithCommas( + numBarTicks + )} samples, ${this.formatter.format(numBarTicks, this.state.sampleRate)}`, + }, + () => { tooltipEl.children[0].innerText = tooltipTitle; } + ); }; mouseOutHandler = () => { - this.canvas.style.cursor = ""; this.setState({ highlightStyle: { - display: "none", + opacity: "0", }, tooltipStyle: { - display: "none", + opacity: "0", }, }); }; diff --git a/webapp/sass/flamebearer.scss b/webapp/sass/flamebearer.scss index 6f5e80c542..028315a4f7 100644 --- a/webapp/sass/flamebearer.scss +++ b/webapp/sass/flamebearer.scss @@ -18,7 +18,9 @@ .flamegraph-tooltip { max-width: 80%; - position: absolute; + position: fixed; + top: 0; + left: 0; pointer-events: none; background: #ffffff; white-space: nowrap; @@ -37,9 +39,13 @@ } .flamegraph-highlight { - position: absolute; + position: fixed; + top: 0; + left: 0; pointer-events: none; background: #ffffff40; // this makes it so that text doesn't get dimmer mix-blend-mode: overlay; + will-change: left, top; + opacity: 0; } From 948f475d067523bacc58ce4724dc47748eb40f1b Mon Sep 17 00:00:00 2001 From: Dmitry Filimonov Date: Tue, 29 Jun 2021 16:48:19 -0700 Subject: [PATCH 2/5] adds fps indicator --- package.json | 1 + webapp/javascript/index.jsx | 9 +++++++++ yarn.lock | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/package.json b/package.json index 3a4e3946c4..2af1639ac1 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "react-datepicker": "^3.4.1", "react-dom": "^16.13.1", "react-flot": "^1.3.0", + "react-fps-stats": "^0.1.3", "react-keybind": "^0.8.1", "react-modal": "^3.12.1", "react-outside-click-handler": "^1.3.0", diff --git a/webapp/javascript/index.jsx b/webapp/javascript/index.jsx index 779a6ee745..cc94d965da 100644 --- a/webapp/javascript/index.jsx +++ b/webapp/javascript/index.jsx @@ -4,6 +4,7 @@ import React from "react"; import { Provider } from "react-redux"; import { ShortcutProvider } from "react-keybind"; import { Router, Switch, Route } from "react-router-dom"; +import FPSStats from "react-fps-stats"; import store from "./redux/store"; import PyroscopeApp from "./components/PyroscopeApp"; @@ -12,6 +13,13 @@ import Sidebar from "./components/Sidebar"; import history from "./util/history"; +let showFps = false; +try { + showFps = window.localStorage.getItem("showFps"); +} catch(e) { + console.error(e); +} + ReactDOM.render( @@ -27,6 +35,7 @@ ReactDOM.render( + { showFps ? : "" } , document.getElementById("root") ); diff --git a/yarn.lock b/yarn.lock index fc48ccd777..2327542740 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7807,6 +7807,11 @@ react-flot@^1.3.0: deep-equal "^1.0.1" jquery "^3.1.1" +react-fps-stats@^0.1.3: + version "0.1.3" + resolved "https://registry.yarnpkg.com/react-fps-stats/-/react-fps-stats-0.1.3.tgz#c6adbbe27d9de5436730a5eb0136780df78f588d" + integrity sha512-/HS2dOP20L8X8Q+iHqQI1T5txXIL53yRt72iHuk2a2tIvhIKKDWfgCadBW2wwYkSiDTtPKe0cRwoGXVumrPy3A== + react-is@^16.13.1, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.6, react-is@^16.9.0: version "16.13.1" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" From 262fea37e609d41d37f0220605ede25aa151f672 Mon Sep 17 00:00:00 2001 From: Dmitry Filimonov Date: Tue, 29 Jun 2021 16:53:43 -0700 Subject: [PATCH 3/5] speeds up rendering by skipping react --- .../components/FlameGraphRenderer.jsx | 61 +++++++++---------- webapp/sass/flamebearer.scss | 10 ++- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/webapp/javascript/components/FlameGraphRenderer.jsx b/webapp/javascript/components/FlameGraphRenderer.jsx index 78a61e4d78..fb23baacd3 100644 --- a/webapp/javascript/components/FlameGraphRenderer.jsx +++ b/webapp/javascript/components/FlameGraphRenderer.jsx @@ -68,6 +68,7 @@ class FlameGraphRenderer extends React.Component { flamebearer: null, }; this.canvasRef = React.createRef(); + this.highlightRef = React.createRef(); this.tooltipRef = React.createRef(); this.currentJSONController = null; } @@ -421,43 +422,35 @@ class FlameGraphRenderer extends React.Component { this.graphWidth ); + const highlightEl = this.highlightRef.current; const tooltipEl = this.tooltipRef.current; const numBarTicks = level[j + 1]; const percent = formatPercent(numBarTicks / this.state.numTicks); - - // a little hacky but this is here so that we can get tooltipWidth after text is updated. const tooltipTitle = this.state.names[level[j + 3]]; - this.setState( - { - highlightStyle: { - opacity: 1, - transform: `translate(${this.canvas.offsetLeft + x}px, ${this.canvas.offsetTop + y}px)`, - width: `${sw}px`, - height: `${PX_PER_LEVEL}px`, - }, - tooltipStyle: { - opacity: 1, - transform: `translate(${e.clientX+12}px, ${e.clientY+12}px)`, - }, - tooltipTitle, - tooltipSubtitle: `${percent}, ${numberWithCommas( - numBarTicks - )} samples, ${this.formatter.format(numBarTicks, this.state.sampleRate)}`, - }, - () => { tooltipEl.children[0].innerText = tooltipTitle; } - ); + // Before you change all of this to React consider performance implications. + // Doing this with setState leads to significant lag. + // See this issue https://github.com/pyroscope-io/pyroscope/issues/205 + // and this PR https://github.com/pyroscope-io/pyroscope/pull/266 for more info. + highlightEl.style.opacity = 1; + highlightEl.style.left = `${this.canvas.offsetLeft + x}px`; + highlightEl.style.top = `${this.canvas.offsetTop + y}px`; + highlightEl.style.width = `${sw}px`; + highlightEl.style.height = `${PX_PER_LEVEL}px`; + + tooltipEl.style.opacity = 1; + tooltipEl.style.left = `${e.clientX+12}px`; + tooltipEl.style.top = `${e.clientY+12}px`; + + tooltipEl.children[0].innerText = tooltipTitle; + tooltipEl.children[1].innerText = `${percent}, ${numberWithCommas( + numBarTicks + )} samples, ${this.formatter.format(numBarTicks, this.state.sampleRate)}`; }; mouseOutHandler = () => { - this.setState({ - highlightStyle: { - opacity: "0", - }, - tooltipStyle: { - opacity: "0", - }, - }); + this.highlightRef.current.style.opacity = "0"; + this.tooltipRef.current.style.opacity = "0"; }; updateSortBy = (newSortBy) => { @@ -565,14 +558,16 @@ class FlameGraphRenderer extends React.Component { -
+
-
{this.state.tooltipTitle}
-
{this.state.tooltipSubtitle}
+
+
) diff --git a/webapp/sass/flamebearer.scss b/webapp/sass/flamebearer.scss index 028315a4f7..51666438c8 100644 --- a/webapp/sass/flamebearer.scss +++ b/webapp/sass/flamebearer.scss @@ -18,7 +18,7 @@ .flamegraph-tooltip { max-width: 80%; - position: fixed; + position: absolute; top: 0; left: 0; pointer-events: none; @@ -28,7 +28,10 @@ border-radius: 4px; padding: 3px 5px; color: #333; + will-change: left, top, opacity; font-size: 12px; + opacity: 0; + // transition: left 0.05s ease-out, top 0.05s ease-out; } .flamegraph-tooltip-name { @@ -39,13 +42,14 @@ } .flamegraph-highlight { - position: fixed; + position: absolute; top: 0; left: 0; pointer-events: none; background: #ffffff40; // this makes it so that text doesn't get dimmer mix-blend-mode: overlay; - will-change: left, top; + will-change: left, top, opacity; opacity: 0; + // transition: opacity 0.05s ease-out; } From 1afe5327dc26b3c91579c3de606d5e3ec8425c02 Mon Sep 17 00:00:00 2001 From: Dmitry Filimonov Date: Tue, 29 Jun 2021 16:58:16 -0700 Subject: [PATCH 4/5] cleanup --- webapp/sass/flamebearer.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/webapp/sass/flamebearer.scss b/webapp/sass/flamebearer.scss index 51666438c8..c02249c416 100644 --- a/webapp/sass/flamebearer.scss +++ b/webapp/sass/flamebearer.scss @@ -31,7 +31,6 @@ will-change: left, top, opacity; font-size: 12px; opacity: 0; - // transition: left 0.05s ease-out, top 0.05s ease-out; } .flamegraph-tooltip-name { @@ -51,5 +50,4 @@ mix-blend-mode: overlay; will-change: left, top, opacity; opacity: 0; - // transition: opacity 0.05s ease-out; } From 8c21177cf867625b38c735f09cf4a8b9d9c13cd5 Mon Sep 17 00:00:00 2001 From: Dmitry Filimonov Date: Tue, 29 Jun 2021 16:59:24 -0700 Subject: [PATCH 5/5] adds a comment --- webapp/javascript/index.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/webapp/javascript/index.jsx b/webapp/javascript/index.jsx index cc94d965da..27f88aa8c1 100644 --- a/webapp/javascript/index.jsx +++ b/webapp/javascript/index.jsx @@ -15,6 +15,8 @@ import history from "./util/history"; let showFps = false; try { + // run this to enable FPS meter: + // window.localStorage.setItem("showFps", true); showFps = window.localStorage.getItem("showFps"); } catch(e) { console.error(e);