vaadin / flow-components

Java counterpart of Vaadin Web Components
100 stars 66 forks source link

Constant browser memory growth in Vaadin Charts (specifically, type LINE) #6383

Open jonathanjt opened 3 months ago

jonathanjt commented 3 months ago

Description

Updating an existing chart data series via DataSeries::add( item, true, true ), with Push enabled, causes a growing memory consumption - Chrome DevTools profiler suggests listeners are being added and not being cleaned up when the data points are shifted.

Expected outcome

Would expect memory to remain roughly constant as new data points are added

Minimal reproducible example

v23-chart-leak.zip

Steps to reproduce

Run the provided demo - in creates a chart with some settings ( similar to our production usage where we spotted the issue, in case the particular settings matter ), and sets a timer to update the chart once per second with new data. Shortening the interval will make the growth quite obvious: image

Environment

Vaadin version(s): 23.5 OS: Linux

Browsers

Chrome

javier-godoy commented 3 months ago

As a lousy workaround , I introduced a call to drawchart(true) every some iterations. It seems to release the listeners.

        AtomicInteger iterations = new AtomicInteger();

        Timer timer = new Timer();
        timer.scheduleAtFixedRate(new TimerTask() {
            @Override
            public void run() {
                if( ui == null ) {
                  return;
                }
                ui.access( () -> {
                    usage_memory_series.add( new DataSeriesItem(
                            Instant.now(), new Random().nextInt(101)), true, true);
                    usage_network_series.add( new DataSeriesItem(
                            Instant.now(), new Random().nextInt(101)), true, true);
                    long t = System.nanoTime();
                    if (iterations.incrementAndGet()==100) {
                        iterations.set(0);
                        usage_chart.drawChart(true);
                    }
                });
            }
        }, 0, 10);

image

OTOH drawChart() which is equivalent to drawChart(false) does not release listeners: image

jonathanjt commented 3 months ago

I my testing, usingdrawChart(true) does release some listeners and free up some memory, but even adding a drawChart(true) after every update still incurs some memory growth, so that slows the process, but doesn't completely work-around the issue.

DiegoCardoso commented 3 months ago

This looks like an issue on Highcharts side, as I was able to reproduce what I believe is the same issue in their latest version.

I reported it here, let's see if they confirm it.

DiegoCardoso commented 3 months ago

As per this comment from one of their maintainers, it has been confirmed that this issue originates on Highcharts.

As usual in this case, we will wait for their fix, and see if it's possible to patch it into our component since Highcharts usually don't backport bug fixes to older versions.

TatuLund commented 1 day ago

I would assume the fix: https://github.com/vaadin/web-components/pull/7785 will give a new workaround to this issue. Instead of just updating Chart data, throw Chart instance away and replace it by new one?