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

Memory leak in all most chart types #1996

Open
ekhaidarov opened this issue Jun 10, 2015 · 35 comments
Open

Memory leak in all most chart types #1996

ekhaidarov opened this issue Jun 10, 2015 · 35 comments
Assignees

Comments

@ekhaidarov
Copy link

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
Copy link

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
Copy link
Author

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
Copy link
Author

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

@dlaliberte
Copy link

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
Copy link
Author

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
Copy link

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
Copy link
Author

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?

@dlaliberte dlaliberte self-assigned this Jun 12, 2015
@ekhaidarov
Copy link
Author

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

@dlaliberte
Copy link

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 [email protected]
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
#1996 (comment)
.

Daniel LaLiberte https://plus.google.com/100631381223468223275?prsrc=2 -
978-394-1058
[email protected] [email protected] 5CC, Cambridge MA
[email protected] [email protected] 9 Juniper Ridge
Road, Acton MA

@ekhaidarov
Copy link
Author

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

@dlaliberte
Copy link

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
Copy link
Author

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

@dlaliberte
Copy link

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 [email protected]
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
#1996 (comment)
.

Daniel LaLiberte https://plus.google.com/100631381223468223275?prsrc=2 -
978-394-1058
[email protected] [email protected] 5CC, Cambridge MA
[email protected] [email protected] 9 Juniper Ridge
Road, Acton MA

@ekhaidarov
Copy link
Author

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

@dlaliberte
Copy link

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

On Wed, Jul 15, 2015 at 3:22 PM, ekhaidarov [email protected]
wrote:

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


Reply to this email directly or view it on GitHub
#1996 (comment)
.

Daniel LaLiberte https://plus.google.com/100631381223468223275?prsrc=2 -
978-394-1058
[email protected] [email protected] 5CC, Cambridge MA
[email protected] [email protected] 9 Juniper Ridge
Road, Acton MA

@ekhaidarov
Copy link
Author

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

@dlaliberte
Copy link

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
Copy link
Author

Hi there,

Curious if there is an update on the item.

@dlaliberte
Copy link

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
Copy link
Author

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
Copy link

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
Copy link

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
Copy link

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

@JamesBurtonBCM
Copy link

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
Copy link
Author

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

@dlaliberte
Copy link

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
Copy link
Author

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

@dlaliberte
Copy link

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
Copy link
Author

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

@ekhaidarov
Copy link
Author

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

@chc88
Copy link

chc88 commented Mar 31, 2016

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

@dlaliberte
Copy link

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

@tedskis
Copy link

tedskis commented Dec 8, 2016

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
Copy link

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

@ecthros
Copy link

ecthros commented Jul 20, 2018

#2650 fixes this issue if anyone is still having it.

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

No branches or pull requests

7 participants