Skip to content

[BUGFIX] Dashboard: remove unmountOnExit flag of the row collapse#80

Open
shahrokni wants to merge 1 commit intomainfrom
fix/row_performance_issue
Open

[BUGFIX] Dashboard: remove unmountOnExit flag of the row collapse#80
shahrokni wants to merge 1 commit intomainfrom
fix/row_performance_issue

Conversation

@shahrokni
Copy link
Contributor

@shahrokni shahrokni commented Mar 3, 2026

relates to perses/perses#3448

⚠️ I would like to have the approval from both @jgbernalp and @AntoineThebaud

Description

This small change makes the heavy dashboards open/close collapses 10X faster!

I drilled down and found out that when row -> collapse -> unmountOnExit is set to true and the user closes and opens the collapse again, (VERY LIKELY) two things happen. (Based on the browser profiling metrics)

  • During open the children hierarchy of the collapsed row are rendered from scratch due to unmountOnExit
  • Tanstack tries to find the results of this query from the cache which is a very heavy process. You can check the network and see that there is no network traffic. So, all data is retrieved from the cache.

Considering the volume of data downloaded for a huge dashboard like nodexporter, retrieving the data from the cache itself is a huge time consuming task! This task alone takes more than 2000 ms

Confusion 💀 Open Scary Question

Why have we set unmountOnExit to true?! Here, the only benefit of collapse is to hide unnecessary info. Why should it destroy the the only and immediate child?! What is the advantage?

Tested corner cases

  • refresh when the group is open
  • refresh when the group is closed and then open (should have been updated)
  • Suggested by @Gladorme It would be nice to test on a dashboard with 100+ panels, and see what happens

Test instruction 🧪

You can find the details of the test instruction here
perses/perses#3448

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.
  • E2E tests are stable and unlikely to be flaky.
    See e2e docs for more details. Common issues include:
    • Is the data inconsistent? You need to mock API requests.
    • Does the time change? You need to use consistent time values or mock time utilities.
    • Does it have loading states? You need to wait for loading to complete.

@Gladorme
Copy link
Member

Gladorme commented Mar 4, 2026

Yeah, as told in private. The unMount has been probably added for perf reasons (https://mui.com/material-ui/react-accordion/#performance). To remove some nodes from DOM, if panel group has a lot of panels in it . That need to be tested too.

@shahrokni
Copy link
Contributor Author

Yeah, as told in private. The unMount has been probably added for perf reasons (https://mui.com/material-ui/react-accordion/#performance). To remove some nodes from DOM, if panel group has a lot of panels in it . That need to be tested too.

Yes it makes sense and I will provide a test. Thank you for this great suggestion.

Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com>
@shahrokni shahrokni force-pushed the fix/row_performance_issue branch from 18a9f66 to a3e9ef7 Compare March 5, 2026 12:51
@shahrokni
Copy link
Contributor Author

shahrokni commented Mar 5, 2026

@Gladorme
Tested with a group panel of 100 individual panels it is still way faster than the previous state. With 100 panels it took max 2000 ms to open the collapse. Currently it takes + 8000ms to open a collapse. I see no advantage setting that flag to true.

To tests I used the following Go DaC

package main

import (
	"flag"
	"fmt"

	"github.com/perses/perses/go-sdk"
	"github.com/perses/perses/go-sdk/dashboard"
	"github.com/perses/perses/go-sdk/panel"
	panelgroup "github.com/perses/perses/go-sdk/panel-group"
	promDs "github.com/perses/plugins/prometheus/sdk/go/datasource"
	"github.com/perses/plugins/prometheus/sdk/go/query"
	timeSeriesPanel "github.com/perses/plugins/timeserieschart/sdk/go"
)

func main() {
	flag.Parse()
	exec := sdk.NewExec()
	groupOptions := []panelgroup.Option{
		panelgroup.PanelsPerLine(2),
	}

	for i := 1; i <= 100; i++ {
		panelTitle := fmt.Sprintf("simple up query #%d", i)

		groupOptions = append(groupOptions, panelgroup.AddPanel(panelTitle,
			timeSeriesPanel.Chart(),
			panel.Title(panelTitle),
			panel.AddQuery(
				query.PromQL("up"),
			),
		))
	}

	builder, buildErr := dashboard.New("TESTDASHBAORD",
		dashboard.ProjectName("TESTPROJECT"),

		dashboard.AddPanelGroup("100 panels",
			groupOptions...,
		),

		dashboard.AddDatasource("promLocal",
			promDs.Prometheus(promDs.HTTPProxy("http://localhost:9090")),
		),
	)

	exec.BuildDashboard(builder, buildErr)
}

@Gladorme
Copy link
Member

Gladorme commented Mar 6, 2026

After, testing with 10 panel groups of 100 panels, unmountOnExit has a big impact (good impact). The tooltip is way smoother, when 1 group is open and other closed.

I am thinking Activity could help here, but it's React 19.2 feature :/ (https://react.dev/reference/react/Activity)

My conclusion, if dashboard has:

  • lot of group with few panels: unmountOnExit is less useful
  • few group with lot of panels: unmountOnExit is useful
  • lot of group with lot of panels: unmountOnExit is very useful
  • few group and few panel: unmountOnExit is not useful

No real data to confirm except "fluidity" of tooltip and no idea what is really a "lot of panel" (here I tried with 100 panels, maybe something nobody will do). Not expert of UI profiling and I have issue to use React Devtools on Perses :(

Tweaked a little bit your DaC:

package main

import (
	"flag"
	"fmt"

	"github.com/perses/perses/go-sdk"
	"github.com/perses/perses/go-sdk/dashboard"
	"github.com/perses/perses/go-sdk/datasource"
	"github.com/perses/perses/go-sdk/panel"
	panelgroup "github.com/perses/perses/go-sdk/panel-group"
	promDs "github.com/perses/plugins/prometheus/sdk/go/datasource"
	"github.com/perses/plugins/prometheus/sdk/go/query"
	timeSeriesPanel "github.com/perses/plugins/timeserieschart/sdk/go"
)

func main() {
	flag.Parse()
	exec := sdk.NewExec()
	groupOptions := []panelgroup.Option{
		panelgroup.PanelsPerLine(2),
	}

	panels := 100

	for i := 1; i <= panels; i++ {
		panelTitle := fmt.Sprintf("simple up query #%d", i)

		groupOptions = append(groupOptions, panelgroup.AddPanel(panelTitle,
			timeSeriesPanel.Chart(),
			panel.Title(panelTitle),
			panel.AddQuery(
				query.PromQL("up"),
			),
		))
	}

	builder, buildErr := dashboard.New(fmt.Sprintf("%d-panels", panels),
		dashboard.ProjectName("q"),

		dashboard.AddPanelGroup("Group 1",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 2",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 3",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 4",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 5",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 6",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 7",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 8",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 9",
			groupOptions...,
		),

		dashboard.AddPanelGroup("Group 10",
			groupOptions...,
		),

		dashboard.AddDatasource("promLocal",
			promDs.Prometheus(promDs.HTTPProxy("https://prometheus.demo.prometheus.io")),
			datasource.Default(true),
		),
	)

	exec.BuildDashboard(builder, buildErr)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants