Memory on acceptance tests on ember 1.13.10+ #12618
Comments
|
this one seems related also #12490 and I have been having similar issues with my app |
|
I've also been struggling with this on 1.13.10. No idea if this would be helpful, but here is my timeline and heap profile. |
|
@lmcardle it is unfortunately not at all helpful, the only thing that is valuable is actual examples demonstrating leaks. |
yes, heap snapshot, find the retainers and address them. |
|
I took a look, although there may be more leaks. I believe I found a v8 bug. It appears to be leaking due to an inlined function preserving its context... After some digging, I found:
Which seems to indicate the The following stack appears inlined (causing the grief).
I then pinged @mraleph on twitter, who recommended i disabling inlining and try again. Manually preventing this specific set of functions from inlining also prevents the leak. hacky patch And the container leak I noticed went away... cc @dgeb
|
|
FYI: when searching for memory leaks, having examples that create excessive noise only creates distractions and make finding the source of the leak more difficult. The key is finding the minimal setup required to cause a leak, basically deleting parts of the example until the leak stops, and following that process until only a concise example remain. After deleting nearly everything the leak was still obvious, and made for a much easier time debugging. Ultimate the leak appears to be a v8 bug, but several work-arounds are possible. With one of those work-arounds, full re-instating the provided tests also no longer leak containers. |
…ry / resolver access. This fix changes the expectations of Registry to accept a `resolver` that’s an object with a `resolve` method instead of a straight function. This allows us to avoid closing over access to a resolver object inside a function. It also allows us to avoid setting functions which shadow prototype functions unnecessarily. Setting `Registry#resolver` to a function is still allowed through a constructor option. The `resolver` function will be converted into an object with a `resolve` method and will result in a single deprecation warning. This fix also eliminates the need for application instances to override their registry’s `normalizeFullName` and `makeToString` methods. Instead, the fallback registry will be checked when evaluating these methods before returning the defaults. Again, this avoids the need to override instance functions that shadow prototype functions. [Fixes emberjs#12618]
|
This should have been addressed by #12666, but I'm reopening so that we can confirm. |
|
Is this fix available for 1.13? |
…ry / resolver access. This fix changes the expectations of Registry to accept a `resolver` that’s an object with a `resolve` method instead of a straight function. This allows us to avoid closing over access to a resolver object inside a function. It also allows us to avoid setting functions which shadow prototype functions unnecessarily. Setting `Registry#resolver` to a function is still allowed through a constructor option. The `resolver` function will be converted into an object with a `resolve` method and will result in a single deprecation warning. This fix also eliminates the need for application instances to override their registry’s `normalizeFullName` and `makeToString` methods. Instead, the fallback registry will be checked when evaluating these methods before returning the defaults. Again, this avoids the need to override instance functions that shadow prototype functions. [Fixes #12618] (cherry picked from commit 529bfc3)
@shen6653 I don't know if it has been backported, we would likely accept a PR doing so. |
|
It seems like this just needs confirmation against the testing repo provided in the original ticket. |
|
Forked the original leaker repo and upgraded to Ember 2.3.2. The memory leak issue seems to have been fixed for this version as part of: #12666 Performant tests timeline (2.3.2) Original tests timeline (1.13.10) |
to be pedantic, we are working around a v8 bug in that PR |
|
I just tested this again with the latest ember/ember-cli and looks so much better !!! Memory doesn't increase between acceptance tests anymore Thanks for the constant improvements ! |
original pr emberjs#12666 fixes emberjs#12618
|
Per comments above, it looks like this there have been improvements. I'll close this for now, but please comment if we should re-open it. |
|
I would love to see a fix done for 1.13.x I'm currently updating an app (quit big) from 1.12.2 to 2.x, and passing by 1.13 to fix all deprecation warning is a most. |
|
Is there any way to manually clear memory between acceptance tests run? I'm also interested in this. I doubt any fix will be backported to 1.13 and it's better to upgrade to Ember 2, but maybe there's some workaround? |
|
@kuzirashi You could do like me. |
test repo: https://github.com/calderas/leaky-mcleakerson
After upgrading our app to ember 1.13 chrome crashes when running Acceptance Tests
Following the same approach as #11748 I ran some tests on a simple app to compare different versions of ember
Observations:
Questions:
Memory comparison
Looking at the timeline seems like event listeners remain after tests have completed and detached elements show up on heap profiles.
test repo: https://github.com/calderas/leaky-mcleakerson
The text was updated successfully, but these errors were encountered: