Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logs Panel: Refactor style generation to improve rendering performance #62599

Merged
merged 13 commits into from
Feb 1, 2023

Conversation

matyax
Copy link
Contributor

@matyax matyax commented Jan 31, 2023

What is this feature?

This PR refactors how we generate and share styles in logs-related components. The main changes are:

  • Parse styles shared by rows and other components once in LogsRow, then pass them as a prop.
  • The dynamic css that sets the row color related with the level of a log has been simplified and delegated to getLogLevelStyles().
  • Individual getStyles() calls made by children components of a LogRow have been moved to getLogRowStyles().

A minor change included in this PR is a TimeZone import deprecation fix.

Which issue(s) does this PR fix?:

Part of #60687
Part of #63030

Special notes for your reviewer:

The tests for this PR have been performed in the Explore page.

The changes in this branch have been validated using the browser profiler and the React profiler before submission, to ascertain that there was a performance gain from each change. Individual changes have been also ran through the profiler, including a refactor of LogRow using functional components which was not included because it had no impact.

Avoiding the use of cx() and calls to getLogLevelStyles() have been tested and showed no visible performance improvements.

Measurements in the main branch:

Main profiler

Scripting main

Measurements in this feature branch:

Performance profiler

Scripting performance

The underlying functionality should, obviously, remain intact.

@matyax matyax requested a review from a team as a code owner January 31, 2023 15:23
@matyax matyax requested a review from svennergr January 31, 2023 15:23
@matyax matyax added this to the 9.5.0 milestone Jan 31, 2023
@matyax matyax added add to changelog no-backport Skip backport of PR labels Jan 31, 2023
Copy link
Contributor

@svennergr svennergr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Left a few comments. WDYT about adding the usage of the useStyles2 hook when calling getLogLevelStyles and getLogRowStyles?

public/app/features/logs/components/LogDetails.tsx Outdated Show resolved Hide resolved
public/app/features/logs/components/LogRow.tsx Outdated Show resolved Hide resolved
@matyax
Copy link
Contributor Author

matyax commented Jan 31, 2023

It's worth mentioning that the measurements and tests for this PR were done in the Explore page. Will follow up with what I see in Dashboards.

@matyax matyax force-pushed the matyax/logs-panel-styles-perf branch from 0adc434 to 6b31fc9 Compare February 1, 2023 14:15
@matyax matyax enabled auto-merge (squash) February 1, 2023 14:20
@matyax matyax merged commit c139768 into main Feb 1, 2023
@matyax matyax deleted the matyax/logs-panel-styles-perf branch February 1, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants