Opened 12 years ago
Closed 11 years ago
#10827 closed defect (worksforme)
[patch][ccla] Chart2D with events causes considerable memory leak on render()
Reported by: | Jared Jurkiewicz | Owned by: | Eugene Lazutkin |
---|---|---|---|
Priority: | high | Milestone: | 1.6 |
Component: | Charting | Version: | 1.4.0 |
Keywords: | Cc: | [email protected]…, [email protected]… | |
Blocked By: | Blocking: |
Description (last modified by )
Reported by Masato Noguchi (IBM): When some events are listened by connectToPlot() or actions are added, dojox.charting.Chart2D causes considerable memory leaks on render(). Which is caused by calls of connect() to Shape in dojox.charting.plot2d.Base._connectEvents, which are never disconnected.
The following code actually eliminates the leaks:
dojo.extend(dojox.charting.plot2d.Base, { resetEvents: function(){ dojo.forEach(this._connects, dojo.disconnect); delete this._connects; this.plotEvent({type: "onplotreset", plot: this}); }, _connectEvents: function(shape, o){ if(!this._connects){ this._connects = []; } this._connects.push(shape.connect("onmouseover", this, function(e){ o.type = "onmouseover"; o.event = e; this.plotEvent(o); })); this._connects.push(shape.connect("onmouseout", this, function(e){ o.type = "onmouseout"; o.event = e; this.plotEvent(o); })); this._connects.push(shape.connect("onclick", this, function(e){ o.type = "onclick"; o.event = e; this.plotEvent(o); })); } });
Attachments (4)
Change History (24)
comment:1 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Summary: | Charting: Chart2D with events causes considerable memory leak on render() → [patch][ccla] Chart2D with events causes considerable memory leak on render() |
comment:2 follow-up: 5 Changed 12 years ago by
comment:3 Changed 12 years ago by
Milestone: | tbd → 1.4.2 |
---|
comment:4 Changed 12 years ago by
Milestone: | 1.4.2 → 1.5 |
---|
1.4.2 has already been released. Suggest targeting for 1.5 although we could have a 1.4.3.
comment:5 Changed 12 years ago by
Status: | new → assigned |
---|
comment:6 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:7 Changed 12 years ago by
Cc: | [email protected]… [email protected]… added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Sheng Tian (IBM) reports:
dojox.charting.action2d.Magnify, dojox.charting.action2d.MoveSlice?,dojox.charting.action2d.Shake, dojox.charting.DataChart?, dojox.charting.plot2d.Pie, all have dojo.connect without disconnect.
comment:10 Changed 12 years ago by
hi, I was getting memory leaks on IE 8 so i applied the patch to my dojo lib to see if it will fix the problem. although the memory leak persists. I am not sure if the problem is the patch or something on the approach I am using on the chart (see attached file. chart_leak.html)
Changed 12 years ago by
Attachment: | test_chart2d_leak_test.html added |
---|
comment:11 follow-up: 13 Changed 12 years ago by
Still can see some memory leak on IE8 after applying Tooltip, Magnify, Highlight actions. See attached test file.
comment:12 Changed 12 years ago by
comment:13 Changed 12 years ago by
Replying to shengt:
Still can see some memory leak on IE8 after applying Tooltip, Magnify, Highlight actions. See attached test file.
No wonder --- your test file creates and attaches actions yet never destroys them. All external objects (actions, legends, data stores, and so on) should be destroyed separately --- the same way they were created separately.
comment:14 Changed 12 years ago by
I spent quality time with the updated leak test and with current code I see a sustainable memory level. I tired Chrome, IE6, and IE8.
comment:15 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:16 Changed 12 years ago by
BTW, it is safe to leave JavaScript objects connected using dojo.connect()
. But if a JavaScript object connected to a DOM node, it may cause leaks, and such links should be disconnected with dojo.disconnect()
. That's why actions do not have to disconnect when they connected to a local animation object.
comment:17 follow-up: 18 Changed 12 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I am still seeing memory leaks on IE 8, FF, Chrome. I have similar as chart_leak.html. I have attached file linechart_leak.html.
comment:20 Changed 11 years ago by
Resolution: | → worksforme |
---|---|
Status: | reopened → closed |
I ran linechart_leak.html
using the trunk for one hour on Chrome watching the memory size --- it was clearly bounded from above being on average between 40-80M.
This is something I don't understand about Dojo. If I destroy a widget or remove a DOM element, all event event handlers bound to that widget or DOM element should be unbound. I shouldn't have to worry about it.
See the jQuery doc for the remove() method. They unbound event handlers, but dojo.orphan() is not doing that.
http://api.jquery.com/remove/