Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback (and thanks!) #1

Open
addyosmani opened this issue Aug 28, 2014 · 0 comments
Open

Feedback (and thanks!) #1

addyosmani opened this issue Aug 28, 2014 · 0 comments

Comments

@addyosmani
Copy link

Hey @gonzaloruizdevilla,

I wanted to thank you once again for putting this (excellent) deck together last year. As you may know it inspired me to flesh out the official memory profiling documentation for DevTools to what we have today & also encouraged me put together an up to date deck on profiling for a last-minute event this week.

I wanted to share some feedback based on the latter about a few things I noticed in case you're giving this talk in the future. Hopefully they'll help avoid some of the issues I ran into:

Timers:

In the current version of the slides, there is definitely a leak caused by the timer but it's so small that it's hardly noticeable. You could attempt to do the same inside a loop (like a few have on StackOverflow) as follows:

for (var i = 0; i < 90000; i++) {
    var buggyObject = {
       callAgain: function() {
         var ref = this;
         var val = setTimeout(function() {
            ref.callAgain(); 
         }, 90000);
       }
    }

    buggyObject.callAgain();
    buggyObject = null;
}

After many rounds of attempting to GC this, the memory consumption remains unchanged.
buggyObject instantiates a setTimeout which calls itself, even if you change buggyObject to
= null. Basically, saying you are done with the object and it can be GC'd. The object
will not be garbage collected because there is still a reference to it in the setTimeout().

Closures:

In the current version, largeStr will attempt to chunk up memory until a is out of scope. I think the messaging around eval might be confusing here too (it's evil, but in this case the wrong tool for the job). That aside, one could rewrite it so that you demonstrate a larger problem - returning a function that passes an argument.

var a = (function () { 
   var smallStr = 'x',
   largeStr = new Array(1000000).join('x'); 

   return function (n) {
        eval('');
        return smallStr; 
   }; 
}());

Memory Leaks:

For the memory leak example with the gallery, it might be useful to comment somewhere what you consider the solution is for users that are looking back at your deck. For example:

  • There is setInterval(move, 2000); in prepareGallery that will be called every click, and there is no clearInterval. A copy of prepareGallery with its property (elements, function definitions etc) will be in memory each click so we should fix that.
  • You could create a reference to setInterval and before re-creating the interval, clear it if it exists. Something like if(x) clearInterval(x); x = setInterval(move, 2000).

Thanks once again for putting this deck together as well as the examples. I hope the docs and our decks on memory profiling GMail and V8 were at least some use for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant