vuejs / vue

This is the repo for Vue 2. For Vue 3, go to https://github.com/vuejs/core
http://v2.vuejs.org
MIT License
208.03k stars 33.69k forks source link

Dom node leak in todo app #1405

Closed Vorror closed 9 years ago

Vorror commented 9 years ago

I stumbled across an interesting library called drool, which basically finds dom leaks and ran the vuejs todo app through it. And these were the results:

before :{"documents":3,"jsEventListeners":7,"jsHeapSizeUsed":4845608,"nodes":261} after:{"documents":3,"jsEventListeners":7,"jsHeapSizeUsed":4526040,"nodes":361}                              ^
AssertionError: node count should match

code used:

var drool = require('drool');
var assert = require('assert');

var driver = drool.start({
  chromeOptions: 'no-sandbox'
});

drool.flow({
  repeatCount: 100,
  setup: function() {
    driver.get('http://todomvc.com/examples/vue/');
  },
  action: function() {
    driver.findElement(drool.webdriver.By.css('.new-todo')).sendKeys('find magical goats', drool.webdriver.Key.ENTER);
    driver.findElement(drool.webdriver.By.css('.todo-list li')).click();
    driver.findElement(drool.webdriver.By.css('.destroy')).click();
  },
  assert: function(after, initial) {
    console.log("before :" + JSON.stringify(initial) + " after:" + JSON.stringify(after));
    assert.equal(initial.nodes, after.nodes, 'node count should match');
  }
}, driver)

driver.quit();

I don't know if this is implementation issue in the library itself or the how the todo app was written. It may also be worthwhile to add this to the ci build? Since I see a few other frameworks doing it.

yyx990803 commented 9 years ago

To be honest, I highly doubt the accuracy of this methodology, because Chrome's timeline profiling and memory snapshots show inconsistent results. When using the timeline profiler to record node counts, it shows that node counts are increasing, however when I try to identify the "leak" by comparing memory snapshots, there are no DOM leaks found.

The author of drool actually opened an issue about this long time ago, and you can see my comment here: https://github.com/tastejs/todomvc/issues/1354#issuecomment-115070749

And also see Paul Irish's comment here: https://github.com/tastejs/todomvc/issues/1354#issuecomment-133798434 - basically what he is saying is that these "forced garbage sweeps" may not always clean nodes that are in fact already detached.

The only way to track down Node leaks is using the memory snapshots comparison - and when I do so on the TodoMVC app using latest 1.0 beta, the snapshots diffs are so clean there's nothing to even look at:

screen shot 2015-10-10 at 4 45 14 pm

To me the only reasonable explanation is that the snapshots force a cleaner GC than other profiling mechanisms, and it's beyond my control if someone tells me Vue has DOM leaks because they are testing it using a profiling mechanism that can only tell you "there are leaks!!!" but cannot tell you where the leaks are coming from.

Vorror commented 9 years ago

Thanks for clarifying!