Skip to content

[Lens] Suggestion performance#43875

Closed
flash1293 wants to merge 64 commits intoelastic:feature/lensfrom
flash1293:lens/suggestion-performance
Closed

[Lens] Suggestion performance#43875
flash1293 wants to merge 64 commits intoelastic:feature/lensfrom
flash1293:lens/suggestion-performance

Conversation

@flash1293
Copy link
Contributor

Based on #43688 and #43865

Improves the performance of suggestions by not re-running the expression if it didn't change. Also debounces the preview instead of the suggestion to show the user other options as fast as possible.

We could add a fallback to the icon as long as the expression loading hasn't finished here.

chrisdavies and others added 30 commits June 24, 2019 14:10
scale both the number and the label.
Use format hints and field formatters in metric vis
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@flash1293
Copy link
Contributor Author

@chrisdavies @wylieconlon I do not intend to merge this PR as is (note the draft status), but this PR shown an approach to keep the suggestions from re-rendering until something actually changed.

The actual preview renderer is memoized, making it necessary for the expression string to change for re-rendering.

To show new suggestions as early as possible, only the preview component is debounced, not the suggestion list itself.

To keep the expression string stable across multiple state changes, generateId is made deterministic by using a local counter. By resetting the counter before creating the suggestions, the exact same suggestion is produced as long as nothing relevant for the suggestions changed. This comes with some drawbacks, because generateId is not controlled by the frame but the extension itself. Each plugin has to take care of resetting the generator appropriately and has to pass in currently used column ids to avoid duplicates. By moving the id generator to the frame and passing it down into the suggestion functions, the logic about resetting and duplicate-avoidance can be solved there. @chrisdavies would this be possible with your current refactoring work about state handling?

Maybe you have a better idea how to tackle this.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@flash1293
Copy link
Contributor Author

After syncing with @chrisdavies most of these problems will disappear if we expose an addColumn function in the datasource public api the visualization can call. Then the generation of unique ids in a given scope is an implementation detail of the datasource and the visualization never has to do this.

@timroes timroes added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Sep 18, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@flash1293
Copy link
Contributor Author

@chrisdavies Do you plan to refactor the usage of state setting before feature freeze to make this possible or should we find an interim solution?

@flash1293
Copy link
Contributor Author

Closing this as it will get addressed later

@flash1293 flash1293 closed this Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Lens Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants