advice on resolving closure leaking issue

18 views
Skip to first unread message

Brandon Furtwangler

unread,
Apr 23, 2014, 11:24:57 PM4/23/14
to jasmine...@googlegroups.com
Greatings.  

We have a fairly large JS codebase (few hundred js files) with several thousand jasmine 1.3.x specs.  We discovered recently that we were running out of memory on certain devices while running tests due to the way we were writing certain async specs.  It was our own fault (or rather misunderstanding), but it is what it is, and we don't have time to fix all the tests or migrate to 2.0, so I'm looking for a workaround.

the faulty tests look something like this:

it("...", function () {
var instance = new ...;
waitsFor(function () {});
runs(function () { ... (doesnt clear instance) });
});

Imagine instance is a fairly large object (with native resources) and several thousand tests have run.  Jasmine holds onto all suites, which hold onto specs, which hold onto blocks, which (in our case) capture the instance.  Even after the specs have completed, jasmine continues to hold a reference, and after a while we run out of memory (on a device with <512MB available).

Of course if time wasn't a factor we'd fix the specs to clean up properly, but given the circumstances I'm looking for a cheaper short term alternative.

My proposed solution is to modify our fork of jasmine 1.3.x to release the func/latchFunction references after completion for Blocks and WaitsForBlocks respectively.  I'm wondering if there might be any hidden down-sides to this solution. AFAIK we don't have anything in our workflow that would run the specs multiple times without a page refresh, but I'm assuming there is some reason why jasmine holds onto these after completion.  

Thoughts?

-Brandon

Davis W. Frank

unread,
Apr 24, 2014, 2:42:36 AM4/24/14
to jasmine...@googlegroups.com
Would it be simpler to just insert afterEach() functions that clean up your instance vars, and maybe move the references out of the its? That's a lot of work, but in large suites that might be required.

Another approach, which sucks in different ways, would be to split your suite into multiple chunks. Take 10 files at a time and see if you can manage memory better that way.

--dwf


--
You received this message because you are subscribed to the Google Groups "Jasmine Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jasmine-js-de...@googlegroups.com.
To post to this group, send email to jasmine...@googlegroups.com.
Visit this group at http://groups.google.com/group/jasmine-js-dev.
For more options, visit https://groups.google.com/d/optout.



--
thx,
--dwf

Brandon Furtwangler

unread,
Apr 24, 2014, 11:34:15 AM4/24/14
to jasmine...@googlegroups.com
Thanks David,

I've very hesitant to do anything that requires touching every suite.  We have hundreds of them and they do the above pattern with slight variations that would require manually inspecting/fixing every spec (of which we have several thousand).  At some point when we have more time we're going to probably want to upgrade to jasmine 2.0, and that will require undertaking such an effort, but I'd rather not do it twice.

Regarding your second suggestion, if I understand you it sounds like you're saying 'just don't run them all at once'.  While that might avoid the out-of-memory probable, it introduces a number of other problems related to how that impacts our CI and reporting workflows.

I've implemented my proposed solution and it has the nice properties of actually fixing the leak while not requiring changes to every suite/spec, however it's unclear to me what other ramifications such an approach might have (any why jasmine wasn't doing it to begin with).

-Brandon

Brandon Furtwangler

unread,
Apr 24, 2014, 11:36:33 AM4/24/14
to jasmine...@googlegroups.com
p.s. Sorry for calling you 'David', I posted without proof reading :p
Reply all
Reply to author
Forward
0 new messages