[ALOY-642] Memory leak with animation.js fadeIn() and fadeOut()
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | Medium |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2013-04-25T22:13:08.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Alloy 1.2.0, 2013 Sprint 09 |
Components | Runtime |
Labels | qe-testadded |
Reporter | Tony Lukasavage |
Assignee | Tony Lukasavage |
Created | 2013-04-25T21:35:16.000+0000 |
Updated | 2013-08-06T21:23:57.000+0000 |
Description
PR: https://github.com/appcelerator/alloy/pull/113
{quote}
plugged memory leak that was preventing garbage collection of target view.
removed anonymous function and replaced with 'if' condition because anonymous function was retaining a reference to the target view thus the view wasn't getting garbage collected. Tested with Instruments. Note, only tested and corrected fadeIn and fadeOut. Other animation methods here probably also need adjusting.
{quote}
and additional comments from the contributor in the PR:
{quote}
Hi Tony, I personally didn't find it was obvious at all. I just kept stripping away stuff in my project until I could see my object getting GCd when expected. My test got down to something super bare bones and I was able to prove that this was indeed a problem that I patched and observed with Instruments that it resolved the issue. I have a foggy idea on how it's possible, let me break down what you can do to test. A trick with the iOS simulator by the way, with Instruments – I found that if you follow the tutorial on the Appcelerator site (can't remember where) about how to profile memory allocations in iOS it does a good job of getting you started with how to look at stuff in Instruments. However, it's misleading that when you remove a view component from its parent and remove all references to an object – set it to null that this will show you GC collection in Instruments. I found, to kind of 'wake up' instruments so that it would refresh, showing me reduction in objects in memory (basically to watch the GC happen when you expect it to), in the iOS simulator, you have to go Hardware > Simulate Memory Warning. I guess this action forces Instruments to get the latest info from the simulator at which point it will see that indeed there's something that has either just been GCd or that needs GCing and does so. Basically after doing that action in the simulator you can watch the numbers and graph reduce in Instruments. If you use the animation fadeIn or fadeOut methods of alloy and do all that I describe above, the component your'e trying to remove doesn't get GCd. Here's the original alloy fadeIn method:
exports.fadeIn = function(to, duration, finishCallback) {
to && to.animate({
opacity: 1,
duration: duration
}, function() {
finishCallback && finishCallback();
});
};
I saw from the commit log that someone added an anonymous function wrapper around the finishCallback which gets called whether or not user passed in a finishCallback, then finishCallback gets called if it's not null/undefined. From the commit log I can see that this wrapper was added because without it titanium would crash and I believe I tried removing it to corroborate that claim and I seem to remember that being true for me as well. So the wrapper seems harmless enough but I THINK what's happening is because the function is anonymous it doesn't get GCd? It's strange, maybe it's something more low level in titanium that's holding on to that function but yeah, that's the culprit. So, instead my patch I submitted was a more wordy but covers the original problem and the GC problem with an if/else and some somewhat redundant code to call to.animate with or without the second finishCallback param. As for your question about other platforms, I honestly can't remember where I left off testing GC with Android, I'm not sure I tested this specific issue and I'm not as familiar with the tools. The best I could see in Android was when I click the button that says something like 'force GC' to effect the GC I could see some bytes being reduced but it was just like, a general bucket of bytes that get reduced for the whole app, not as granular as with Instruments (maybe it can be done with Android but I don't know how yet). You can try testing this out with a regular UI.View object
{quote}
PR: https://github.com/appcelerator/alloy/pull/113 test app: https://github.com/appcelerator/alloy/tree/master/test/apps/testing/ALOY-642 For functional testing, ensure that the fadeIn() and fadeOut() animations are executing properly at runtime, and also, per the details in the PR link, make sure that the operation of the app is not leaking memory.
Verified as fixed and not leaking memory on both iOS and Android devices. Titanium SDK 3.1.2.v20130806034554 Alloy 1.2.0-alpha6 Appcelerator Studio 3.1.2.201308021524 CLI 3.1.1 Node 0.10.13 Closing.