Huge lag renaming workpad #25070
Labels
bug
Fixes for quality problems that affect the customer experience
Feature:Canvas
impact:needs-assessment
Product and/or Engineering needs to evaluate the impact of the change.
loe:medium
Medium Level of Effort
performance
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
Comments
alexfrancoeur
added
bug
|
Pinging @elastic/kibana-canvas |
|
Current theory is that as the size of the workpad increases, all actions that do not have apply button are slow (not just renaming workpad). It is easiest to see this effect by adding lots of images. There is probably a short term low hanging fruit improvement we can make here and a longer term architectural change needed. We will try to get the lonw hanging fruit fix done asap (possibly 6.6) Some other notes form the conversation: |
|
When I set up why-did-you-update, there's a stream of needless re-renders that happen every time you change a character in the workpad's name.
That stream of output is triggered by deleting a character, and you can see a huge delay before Chrome is able to render the change in the browser. Every line is a needless re-render, and the WorkpadPage just thrashes with every character change.
#31734 makes this a little better (it prevents all the elements in the page from also re-rendering), but there's still a big performance win to be had here by stopping re-renders. |
w33ble
added a commit
that referenced
this issue
Mar 2, 2019
) In an effort to remove the workpad renaming lag in #25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
w33ble
added a commit
to w33ble/kibana
that referenced
this issue
Mar 2, 2019
…stic#31958) In an effort to remove the workpad renaming lag in elastic#25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
w33ble
added a commit
to w33ble/kibana
that referenced
this issue
Mar 2, 2019
…stic#31958) In an effort to remove the workpad renaming lag in elastic#25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
w33ble
added a commit
to w33ble/kibana
that referenced
this issue
Mar 2, 2019
…stic#31958) In an effort to remove the workpad renaming lag in elastic#25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
w33ble
added a commit
that referenced
this issue
Mar 5, 2019
) (#32359) In an effort to remove the workpad renaming lag in #25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
w33ble
added a commit
that referenced
this issue
Mar 5, 2019
) (#32358) In an effort to remove the workpad renaming lag in #25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
w33ble
added a commit
that referenced
this issue
Mar 5, 2019
) (#32357) In an effort to remove the workpad renaming lag in #25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering
monfera
pushed a commit
to monfera/kibana
that referenced
this issue
Mar 8, 2019
…stic#31958) In an effort to remove the workpad renaming lag in elastic#25070, I started hunting down and fixing re-renders that happened with the name change. I was hoping this PR would fix the lag, but I've shifted to some other performance issues that have a much larger impact, so this is just a bunch of re-rendering fixes, which come with some performance gains. Here's the breakdown: - `Workpad`, `WorkpadHeader`, and `FullscreenControl` would always re-render because the hotkey handler function was always re-created - `Workpad` would re-render when the workpad's name changed - We were passing the whole workpad object into the component, so it re-rendered on all sorts of changes - Page title updating moved to middleware so the component doesn't need that value - `AssetManager` would always re-render if the parent re-rendered because it always created a new state value in `mapStateToProps` - make `Workpad` and `Toolbar` pure, they take no props and this helps stop some re-rendering (cherry picked from commit 7d8dbc6)
clintandrewhall
added
the
impact:needs-assessment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There is a strange lag when renaming the workpad and I now have a feeling it has to do with updating the title of the tab. See the gif below
cc: @cqliu1
The text was updated successfully, but these errors were encountered: