elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.81k stars 8.2k forks source link

[Lens] improve Lens performance when using certain form controls #175485

Open drewdaemon opened 8 months ago

drewdaemon commented 8 months ago

Background

Some form controls in Lens can be quite laggy, especially on weaker machines.

https://github.com/elastic/kibana/assets/315764/e5fee04d-239d-4ff3-a46c-c2a301cadc3c

We can investigate the reason the performance is affected. A few suspects:

Inefficient component rendering?

The render thread can get busy when typing in an input.

I noticed that typing in the dimension name input triggers some forced layout reflows:

Screenshot 2024-01-25 at 2 08 41 PM

How often are our expensive component re-rendering (e.g. WorkspacePanel, LayerPanel). Are there some EUI components that are inefficient? Is the chart render process part of the problem?

Cost of state update routines?

State updates can involve calls to several classes. Is or something related slowing us down?

global-store-pic

State syncing patterns?

Lens employs a global state store which accepts state change notifications and broadcasts changes to the rest of the UI. This pattern keeps signifiers across the UI (visualization renderer, other form controls, warning messages) in sync with the current system truth.

Screenshot 2024-01-23 at 3 09 29 PM

A form control changes a piece of state. The rest of the app receives the new global state.

There are various approaches to how these notifications should flow between form controls, intermediate parents (think containing React components), the global store, and the rest of the application. Each has advantages and disadvantages.

Global sync

Screenshot 2024-01-23 at 3 15 19 PM

The form control sends state update notifications directly to the global store and waits for the notification to return before updating its signifiers. This ensures that its signifiers will always match the system truth. If another UI component makes a change to the relevant state, this form control will update to reflect that.

However, there is a performance cost. Because the form control is waiting on a notification from the store before updating, its signifiers will update no faster than the store can perform a state update and notify all subscribers (worst case).

This may cause the control's signifiers to lag the users actions (e.g. user has to wait 200ms for the character to show up in a text input after they press a key).

This is the pattern we use most often in Lens.

Scoped sync

Screenshot 2024-01-23 at 3 24 36 PM

The form control sends state change notifications to an intermediate parent component which broadcasts the notification to its children, including the originator, and passes it along to the global store without waiting for a reply.

The form control is still left waiting on a state update, but it is likely to be faster since there is less work to do when notifying a subset of the application instead of the whole thing. Also, scoped state updates are less likely to involve the complicated routines or even business logic that are sometimes tied to global state updates.

Like with a global sync, the form control will still receive notifications from other UI components in the application, but only if the originator is a child of the intermediate parent.

Uncontrolled

Screenshot 2024-01-24 at 1 11 55 PM

The form control notifies the global store of state changes, but does not subscribe to notifications from the global store. The form control does not wait on the store to update its signifiers, so there will be no delay. However, state changes that originate elsewhere will not be reflected in the form control.

Conditional sync

Screenshot 2024-01-24 at 2 25 18 PM

Each form control (or group of form controls) could subscribe and unsubscribe to store updates depending on its current state. For example, a focused input might temporarily unsubscribe from store updates while the user is interacting with it. Then, when the user is finished, it might renew its subscription. This could prevent the form control from having to wait on store updates when in use (removing lag), while still keeping it in sync with the state when it is not in use.

Debouncing

There's a performance cost to updating the global state. This cost is most visible in syncing patterns that block user actions on a state update, but the cost is present in some form no matter which syncing pattern is employed.

This generally isn't an issue unless many state updates are being made quickly (for example, during typing).

The classic answer to these situations is debouncing—merging multiple state updates, made in quick succession, into a single notification to the global store.

However, this can significantly extend the time it takes the local state of a form control to be committed to the global store. This leaves other components of the application with outdated state for longer periods of time, during which they can themselves make state updates (in some scenarios).

If this occurs and the active form control is subscribed to the global store, the debounced changes can be overwritten with the current global state before they have been committed to the global store.

Our current answer to this in Lens is a React hook called useDebouncedValue.

This hook ignores upstream state notifications if debounced changes are currently in the queue, theoretically ensuring that debounced changes aren't overwritten.

Conclusion

We should re-evaluate our approach in Lens to make sure we're choosing the best possible strategy for each situation.

Even with our custom debounce logic, our form controls can be laggy, especially on computers that don't have as much CPU.

https://github.com/elastic/kibana/assets/315764/e5fee04d-239d-4ff3-a46c-c2a301cadc3c

elasticmachine commented 8 months ago

Pinging @elastic/kibana-visualizations (Team:Visualizations)

dej611 commented 8 months ago

I think we migrate most of our form controls to the debounced flow, but if not, perhaps we should. On the other hand, I think no matter which approach we use, we're going to have lagging unless we optimize the other end of the pipe, which is slow components updating themselves, my opinion.

drewdaemon commented 8 months ago

@dej611 part of the question is whether our current useDebouncedValue hook (global sync + debouncing + some logic to sequence events) is the right strategy — there are others we could try out.

The NameInput in the laggy video above is already using useDebouncedValue.

I think no matter which approach we use, we're going to have lagging unless we optimize the other end of the pipe, which is slow components updating themselves, my opinion.

Can you explain a bit more on this?

dej611 commented 8 months ago

Reusing your initial diagram, I am referring to this:

global-store-pic

drewdaemon commented 8 months ago

Actually, even an uncontrolled form control (no subscription to the global state) lags during state updates because everything is happening on the render thread

https://github.com/elastic/kibana/assets/315764/349178ec-d59a-44ab-92c2-69278d9f8e20

drewdaemon commented 8 months ago

No big surprise that V8.execute is the number-one contributor to input latencies (ref).

drewdaemon commented 8 months ago

Even after removing the chart from the workspace I notice some expensive forced reflows while typing

Screenshot 2024-01-25 at 2 08 41 PM
dej611 commented 8 months ago

That's interesting. In the past I've seen some re-render expensive flows due to some EuiComboBox resize/measurement tasks: https://github.com/elastic/eui/issues/4866

Maybe there's something similar happening there.

timductive commented 8 months ago

The team talked and agree that this is worth a time-boxed technical spike to investigate either specific slow components or any systemic performance issues. @drewdaemon will update the issue to propose specific components to focus on.