Possible Memory Leak #11501
Comments
|
I see built-in helper streams among the leaks: But they would leak if the morphs were leaking. |
|
@mixonic Are you going to be looking into this some more? Would love to help out if I can and learn a little bit more about how to debug these issues. |
|
just a quick guess (not familiar with this code) but i don't see a corresponding unsubscribe to |
|
actually they all look attrMorph retained. |
|
[edit: this still leaks] http://output.jsbin.com/lemexuyudu#/simple something like its Still going to look for the leak though |
|
I have found the problem, or at least found the source of one of the leaks. I need to spend some time tonight yet thinking about which solution puts on a better path... |
stefanpenner
added a commit
to stefanpenner/ember.js
that referenced
this issue
Jul 6, 2015
tomdale
pushed a commit
that referenced
this issue
Jul 7, 2015
This commit resolves a memory leak (#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself.
stefanpenner
pushed a commit
that referenced
this issue
Jul 7, 2015
This commit resolves a memory leak (#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself.
|
the leak demonstrated is fixed |
machty
pushed a commit
to machty/ember.js
that referenced
this issue
Jul 8, 2015
machty
pushed a commit
to machty/ember.js
that referenced
this issue
Jul 8, 2015
This commit resolves a memory leak (emberjs#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself.
machty
pushed a commit
to machty/ember.js
that referenced
this issue
Jul 8, 2015
This commit resolves a memory leak (emberjs#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself.
stefanpenner
added a commit
that referenced
this issue
Jul 9, 2015
(cherry picked from commit 1b91adc)
rwjblue
pushed a commit
that referenced
this issue
Jul 9, 2015
This commit resolves a memory leak (#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself. (cherry picked from commit 0d43d24)
rwjblue
pushed a commit
that referenced
this issue
Jul 9, 2015
This commit resolves a memory leak (#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself. (cherry picked from commit 8574876)
stefanpenner
added a commit
that referenced
this issue
Jul 9, 2015
(cherry picked from commit 1b91adc)
rwjblue
pushed a commit
that referenced
this issue
Jul 9, 2015
This commit resolves a memory leak (#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself. (cherry picked from commit 0d43d24)
rwjblue
pushed a commit
that referenced
this issue
Jul 9, 2015
This commit resolves a memory leak (#11501) in apps running Glimmer. The root cause of the issue was that a flag governing whether components/views remove themselves from their parent was being set incorrectly, so that when the code to cleanup a destroyed view ran, it was not removed from its parent’s `childViews` array. Specifically, three hooks are invoked when a render node is cleared: 1. `env.hooks.willCleanupNode` 2. `Morph#cleanup` 3. `env.hooks.didCleanupNode` Prior to this commit, `willCleanupNode` would blindly set the owner view’s `isDestroyingSubtree` flag if there was a view set in the `env` (i.e., basically always). Sidebar on the `isDestroyingSubtree` flag: this flag is used as a performance optimization. If a view is destroyed, we want to remove that view from its parent’s `childViews` array so that garbage collection can happen. However, it is unnecessary to remove child views from the destroyed view’s `childViews` array; the GC will take care of any clean up when the link between the view hierarchy and the destroyed view is severed. For example, imagine this hypothetical view hierarchy: A / \ B C / \ D E If the render node for view C is destroyed, we need to remove C from A’s array of child views. However, it is unnecessary to remove D or E from C’s child views, because they will be imminently removed by the GC, and temporarily lingering in the child views array does no harm. We accomplish this by setting the `isDestroyingSubtree` flag on the root-most view in a hierarchy (view A in the example above), which we call the owner view. When the render node for C is destroyed, it removes C from A’s child views array and sets the flag to true. When D and E’s render nodes are destroyed, they see that the flag has already been flipped and do not remove their associated views from C’s child views array. The memory leak manifested itself when the a render node that did not have a view associated with it was destroyed, and contained a child render node that _did_ have a view associated. For example: ```handlebars {{#if foo}} {{my-component}} {{/if}} ``` In this case, the `willCleanupNode` hook would erroneously set the flag without actually moving the `my-component` view, because the render node for the `{{if}}` does not know about the component. When the cleanup for `{{my-component}}` finally happens (in Morph#cleanup), the flag has already been set, so the render node incorrectly assumes that its view is part of a subgraph that has already been severed. As far as we can tell, the `willCleanupNode` hook is not doing any cleanup that is not already done in `Morph#cleanup`. We still do need `didCleanupNode` to clear the flag, but can leave individual render node cleanup to the render node itself. (cherry picked from commit 8574876)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I think I've found a memory leak: http://emberjs.jsbin.com/toqiba#/complicated
If you navigate between
simpleandcomplicatedtaking a heap profile each time the memory usage steadily climbs.EmberMorphseems to be getting pretty high on the retained size.The text was updated successfully, but these errors were encountered: