ngTransclude memory leak fix #6181

renz45 · 2014-02-07T19:53:02Z

I've been profiling a one page app using Angular.js and have had a tough time nailing down this memory leak. I'm using Angularjs for a custom template system that outputs data from a content DSL containing educational content. In this system I swap out a lot of templates full of other angular directives which in themselves sometimes compile/transclude content. After a few days of searching I've finally narrowed down one of the larger memory leaks. Using transclude: true in a directive is fine, no problems with leaks, but as soon as ng-include is used within the template I was getting a lot of detached DOM fragments in the Chrome memory profiler.

Here is a sample app which replicates the problem:

<!DOCTYPE html>
<html>
<head>
  <title>angular.js bug test</title>
</head>
<body ng-app="app">
  <first-level></first-level>
</body>
  <script src="angular.js"></script>
  <script>
    app = angular.module('app', []);

    app.directive('ngTranscludeFixed', function(){
      return {
        restrict: 'AC',
        link: function(scope, element, attrs, ctrl, transclude){
          if (!transclude) {
            throw minErr('ngTransclude')('orphan',
                'Illegal use of ngTransclude directive in the template! ' +
                'No parent directive that requires a transclusion found. ' +
                'Element: {0}',
                startingTag(element));
          }

          transclude(function(clone) {
            element.empty();
            element.append(clone);
          });
        }
      }
    });

    app.directive('secondLevel', function(){
      return {
        replace: true,
        restrict: 'E',
        scope: {},
        transclude: true,
        template: "<div><div class='number-2' ng-transclude></div>"
      }
    });

    app.directive('firstLevel', function($compile){
      return {
        replace: true,
        restrict: 'E',
        scope: {},
        template: "<div class='number-1'><button ng-click='switchContents(1)'>One</button><button ng-click='switchContents(2)'>two</button><p></p></div>",
        link: function(scope, element, attrs){
          var content1 = "<second-level>hello world! 1</second-level>"
          var content2 = "<second-level>hello world! 2</second-level>"
          var pEl = element.find('p')
          var childScope = null;

          scope.switchContents = function(number){
            if(childScope) {
              childScope.$destroy();
              childScope = null;
            }

            pEl.empty()
            childScope = scope.$new()
            if(number == 1) {
              pEl.append(angular.element(content1));
            }else{
              pEl.append(angular.element(content2));
            }

            $compile(pEl.contents())(childScope);
          }
        }
      }
    });
  </script>
</html>

So running this using ng-transclude in the secondLevel directive, you can click the buttons a few times to see the transclude working, but when profiled in dev tools we get:

As you can see there are 4 detached Dom node Trees as a result, not a huge deal in this smaller app, but in my larger app it's causing 350+ detached nodes and about 5mb heap increase per template change. This is a fairly large problem for a one page app since we never reload the page.

Now if we swap out ng-transclude with the proposed changes (ng-transclude-fix) we get:

No detached dom nodes!

The tests pass, I'm not entirely sure how you would even test for detached Dom nodes and memory heap size. From my testing, it seems like the act of assigning $transclude to the controller (or scope for that matter) causes the detached nodes. This fix uses the new transclude function included within the link function to handle the transclude, instead of passing the $transclude object from the controller to the linking function. This is functionally the same, and eliminates the leak.

…xes a memory leak when a directive using ng-transclude is compiled

Since we now pass in the transclusion function directly to the link function, we no longer need the old scheme whereby we saved the transclude function injected into the controller for later use in during linking. Additionally, this change may aid in correcting a memory leak of detached DOM nodes (see angular#6181 for details). This commit removes the controller and simplifies ngTransclude. Closes angular#5375 Closes angular#6181

renz45 added 2 commits February 7, 2014 14:49

Rewrite ngTransclude to use the new transclude link function, this fi…

55260d2

…xes a memory leak when a directive using ng-transclude is compiled

Forgot a semicolon

24a5e07

renz45 mentioned this pull request Feb 7, 2014

refactor(ngTransclude): use transclusion function passed in to link rath... #5375

Closed

IgorMinar added the gh: PR label Feb 10, 2014

caitp closed this in 08793a6 Feb 11, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ngTransclude memory leak fix #6181

ngTransclude memory leak fix #6181

renz45 commented Feb 7, 2014

ngTransclude memory leak fix #6181

ngTransclude memory leak fix #6181

Conversation

renz45 commented Feb 7, 2014