google / google-visualization-issues

288 stars 35 forks source link

Memory leak in all most chart types #1996

Open ekhaidarov opened 9 years ago

ekhaidarov commented 9 years ago

There is still a memory leak across various chart types : gauge, ComboChart, LineChart.

I am including sample js/html that will populate gauge on 1 second interval. You will notice that Chrome memory set keeps jumping with every iteration. General draw function is being used that calls clearChart() before every draw operation. Please advise if it can be fixed or how to work around it. Thanks.

google.load("visualization", "1", { packages: ["gauge", "corechart", 'line'] });
        google.setOnLoadCallback(test);

function test()
        {    
            setInterval(function () { onCPU(Math.floor((Math.random() * 100) + 1)); }, 1000);
        }

var gCharts = [];
function drawChart(data, options, divId, fchartType)
        {
            if(gCharts[divId] != null && gCharts[divId] != undefined)
            {
                gCharts[divId].clearChart();
                gCharts[divId] = null;                
            }
            gCharts[divId] = new fchartType(document.getElementById(divId));
            gCharts[divId].draw(data, options);
            data = null;
            options = null;
        }

        // historical data over cpu and bandwidth
        var MAX_HISTORY = 120;
        var cpuHistory = [];
        var tsStartTime = new Date().getTime();

        function onCPU(cpuLoad)
        {
            var data = google.visualization.arrayToDataTable([
              ['Label', 'Value'],
              ['CPU', cpuLoad]]);

            var options = {
                width: 400, height: 120,
                redFrom: 90, redTo: 100,
                yellowFrom: 75, yellowTo: 90,
                minorTicks: 5
            };

            drawChart(data, options, "cpu_div", google.visualization.Gauge);
            data.removeRows(0, data.getNumberOfRows());
            data.removeColumns(0, data.getNumberOfColumns());
            data = null;
            options = null;

            if(cpuHistory.length > MAX_HISTORY)
            {
               cpuHistory.shift(); 
            }   

            cpuHistory[cpuHistory.length] = [
            Math.abs(Math.floor((new Date().getTime() - tsStartTime)/1000)),
            cpuLoad];

            var CPUHistoryData = new google.visualization.DataTable();
            CPUHistoryData.addColumn('number', 'Time');
            CPUHistoryData.addColumn('number', 'CPU');
            CPUHistoryData.addRows(cpuHistory);

            var CPUHistoryOptions = {
              title: 'CPU Load',
              legend: { position: 'none' }
            };

          drawChart(CPUHistoryData, CPUHistoryOptions, 'cpu_history', google.visualization.LineChart);
          CPUHistoryData = null;
          CPUHistoryOptions = null;
        }

 
 <body>
    <div id="cpu_div" style="width: 100px; height: 120px; position: absolute; left: 10px; top: 10px" /> 
    <div id="cpu_history" style="width: 600px; height: 300px; position: absolute; left: 10px; top: 130px;"/>
</body>
dlaliberte commented 9 years ago

Since your cpuHistory array grows each time you redraw, you should expect the memory usage to increase. If you stop growing it after, say, 10 entries, while you continue to redraw, does the memory usage still grow?

There may well be more (or new) memory leaks, so we are very interested in fixing any that occur.

ekhaidarov commented 9 years ago

Thanks for the quick reply. In fact cpuHistory isn't the issue at all. Even if I remove cpuHistory and reset it every time, every draw operation jumps the memory by 50-100k. I left couple of different samples in place, so you can easily see that the problem is consistent across various types of charts. I was going to submit an actual html page, but looks like the system doesn't allow html file uploads. Just wanted to reiterate simply redrawing the gauge without any other controls grows the memory consistently. Can you please advise if there is a work around available or a better technique to manage a real time data updated using the various chart types? Thank you.

ekhaidarov commented 9 years ago

Adding a link to test page you can pick up off my gdrive : https://drive.google.com/open?id=0B0L1wwJBiSdAZFc5Vm1DeVBaaWs&authuser=0

dlaliberte commented 9 years ago

Thanks for your update. I missed that you were shifting the cpuHistory after it reached the max length, so it will not increase in length thereafter. I created a jsfiddle with this code, modified to allow the running to be interrupted: https://jsfiddle.net/dlaliberte/neyhn38h/

Another factor to consider is that the memory can increase for a while before the browser cleans up more completely. I found it useful to go to the Timeline tab and click the 'Collect garbage' icon (the garbage can) between profile snapshots.

If doing a clearChart is not enough, you can also try to remove the container element from the page, and create new chart objects. That way, there should be nothing left in memory still attached to the previous chart. If memory still grows, then there is either a browser memory leak, or Google Charts must be setting some global memory that is not being cleaned up. We try to work around browser memory leaks (by managing event handlers, for example) but there could be more we don't know about.

ekhaidarov commented 9 years ago

I actually profiled the code in Chrome and it looked like the biggest chunks were allocated via dom html elements and SVG elements, second largest was coming from internal structures of the library. Mind you since html content of the test page is static, defines containers only. I couldn't really tell specifically what in the code is eating away memory as the code is obfuscated.

Also wanted to highlight that the problem exists in other browsers too. I specifically tested in QtWebkit engine and the symptoms are identical. Also if you simply disable draw function and watch the update nothing grows, which makes sense, so all the signs are pointing to internal memory management in the lib.

I can try removing the container, but I am afraid it might force browser to re-render and create flickering.

dlaliberte commented 9 years ago

Certainly drawing a chart will initiate memory usage, but the question is who is responsible for cleaning up afterwards, after a clearChart, and especially before the next draw. Memory associated with HTML and SVG elements could be tied up with event handlers that have not been cleaned up, but regardless, a disconnected DOM element that is no longer in any JS scope should be garbage collected. Failing in various ways to do that properly has been the source of most of the memory leaks in many browsers. Working around these memory leaks has fallen to libraries, such as Google Charts, though we should keep the pressure on the browsers to fix their memory leaks.

Google Charts doesn't (or shouldn't) accumulate any memory of previous state, except it does hold on to the state of each drawing so it can compare with the next drawing, and then it throws away the previous state. It's difficult to see where a leak could occur in that process, but it is possible. Previous leaks were all browser leaks that we worked around.

Removing the container and reconstructing your chart object (and datatable object) is just for purposes of hopefully discovering where the leak is coming from. If it works, this could also be a workaround, but you are right that it could result in flickering if the redraw is not fast enough. If you are serious about it, you could set up a second container to draw into, and then quickly swap the drawings, but be sure you only draw in a container that is visible.

ekhaidarov commented 9 years ago

I tried the technique to remove and recreate container element, but no luck whatsoever. Pattern persists memory keeps growing. Even your example in jsfiddle running in Chrome in windows keep steadily growing. I took a profiler snapshot and kept seeing that function z() keeps creating recurrence and allocation. Again code is obfuscated, so can't really tell what is going on inside of it. Wondering if I could get some assistance here?

ekhaidarov commented 9 years ago

Hi @dlaliberte , just curious if you had an update by chance?

dlaliberte commented 9 years ago

It does appear that you have found a memory leak, and I'll have to take time to investigate the cause and find a workaround. Most likely it involves event handlers, and I should be able to figure it out, but it may be a while before I can get to it.

In the meantime, another more drastic workaround is to refresh your page periodically.

On Tue, Jun 16, 2015 at 10:45 AM, ekhaidarov notifications@github.com wrote:

Hi @dlaliberte https://github.com/dlaliberte , just curious if you had an update by chance?

— Reply to this email directly or view it on GitHub https://github.com/google/google-visualization-issues/issues/1996#issuecomment-112455694 .

Daniel LaLiberte https://plus.google.com/100631381223468223275?prsrc=2 - 978-394-1058 dlaliberte@Google.com dlaliberte@google.com 5CC, Cambridge MA daniel.laliberte@GMail.com daniel.laliberte@gmail.com 9 Juniper Ridge Road, Acton MA

ekhaidarov commented 9 years ago

I tried refreshing the page and it is not helping. Somehow objects are cached still and the memory footprint continues to grow.

dlaliberte commented 9 years ago

Thanks for trying the page refresh. There is a similar issue at the application level, where the memory for the whole application can grow as much as the operating system allows, until the OS forces the application to garbage collect. In Windows, years ago, I found that just minimizing an IE window would cause it to garbage collect more completely, thus hiding the fact that there was a memory leak that persists until that point.

ekhaidarov commented 9 years ago

Can you please advise if you think there will be a resolution over the next week or so?

dlaliberte commented 9 years ago

There will not be a resolution in the next week or so. We're dealing with several other issues that are higher priority at the moment. While not many people are redrawing charts enough to notice memory leaks, we still consider it a serious problem that will be addressed relatively soon, I'd guess in the next couple months.

On Thu, Jun 25, 2015 at 11:14 AM, ekhaidarov notifications@github.com wrote:

Can you please advise if you think there will be a resolution over the next week or so?

— Reply to this email directly or view it on GitHub https://github.com/google/google-visualization-issues/issues/1996#issuecomment-115288755 .

Daniel LaLiberte https://plus.google.com/100631381223468223275?prsrc=2 - 978-394-1058 dlaliberte@Google.com dlaliberte@google.com 5CC, Cambridge MA daniel.laliberte@GMail.com daniel.laliberte@gmail.com 9 Juniper Ridge Road, Acton MA

ekhaidarov commented 9 years ago

Just wondering if by chance there is an update on this bug.

dlaliberte commented 9 years ago

I haven't had any spare time to investigate the memory leak.

On Wed, Jul 15, 2015 at 3:22 PM, ekhaidarov notifications@github.com wrote:

Just wondering if by chance there is an update on this bug.

— Reply to this email directly or view it on GitHub https://github.com/google/google-visualization-issues/issues/1996#issuecomment-121720002 .

Daniel LaLiberte https://plus.google.com/100631381223468223275?prsrc=2 - 978-394-1058 dlaliberte@Google.com dlaliberte@google.com 5CC, Cambridge MA daniel.laliberte@GMail.com daniel.laliberte@gmail.com 9 Juniper Ridge Road, Acton MA

ekhaidarov commented 9 years ago

Sorry to probe again, just curious if there is an update by chance?

dlaliberte commented 9 years ago

There will be more updates to the backlog of bugs when we finish with the currently pending releases. More progress on that front momentarily...

ekhaidarov commented 9 years ago

Hi there,

Curious if there is an update on the item.

dlaliberte commented 9 years ago

We are working on this now. We found the cause of a large class of problems, and the fix is fairly straightforward, so this should show up in v43. (No definite timeline on when v43 will show up, but hopefully a couple weeks.)

ekhaidarov commented 9 years ago

That's great news! Thank you. We are shipping a release ourselves in couple of weeks, I am curious if by chance I could get an early copy when you feel it is more less stable?

JamesBurtonBCM commented 9 years ago

Hello, I seem to be having similar issues to those described above. I'm redrawing a chart every second or two..I'm calling clearchart(), then draw() but looking at the elements in chrome I can see numerous divs added to my body element of the type.

CurrentPosition

They appear to get added on every redraw..When are the memory fixes being released and is this one of the known issues?

JamesBurtonBCM commented 9 years ago

the example divs I pasted didn't get included...here they are.

<div style="position: absolute; display: none;"><div style="padding: 1px; border: 1px solid infotext; font-size: 15px; margin: 15px; font-family: Arial; background: infobackground;">CurrentPosition</div></div>
dlaliberte commented 9 years ago

We are about to push out v43 now, which will contain fixes for memory leaks.

JamesBurtonBCM commented 9 years ago

ok that's good news. hope all the issues are addressed because google charts look pretty cool and I'd like to start using them...

looks like the div issue is related to having a legend, setting it to 'none' stops that issue. Not ideal but unfortunately there is still a memory leak anyway..

ekhaidarov commented 9 years ago

@dlaliberte Can you please advise when v43 is fully published?

dlaliberte commented 9 years ago

The schedule is, generally, after the candidate release, we will work on resolving issues that come up for the next couple weeks, and if all goes well, then we publish. The candidate release for v43 will hopefully be out early next week.

ekhaidarov commented 9 years ago

Is this the correct place where schedule is published : https://developers.google.com/chart/interactive/docs/release_notes ?

dlaliberte commented 9 years ago

The release notes are published there, but the announcements are posted to the Google groups: https://groups.google.com/forum/#!forum/google-visualization-api

ekhaidarov commented 8 years ago

It seems with version 43 the memory leak is not fixed.

ekhaidarov commented 8 years ago

Looks like google.charts.Line still has the memory leak.

chc88 commented 8 years ago

It also seems to happen in version 44. Any update on this issue?

dlaliberte commented 8 years ago

We haven't had time to investigate memory leaks in the last few versions.

tedskis commented 7 years ago

Has a fix been found? I am using angular2 and if I update the data going into my charts more than a few times my browser crashes.

yuhao-nyc commented 7 years ago

Also have memory leak issues with line charts. I was reading all the above comments is there any updates on this?

ecthros commented 6 years ago

2650 fixes this issue if anyone is still having it.