LearnBoost / tobi

Tobi: Expressive server-side functional testing with jQuery
408 stars 33 forks source link

Performance #54

Open mhemesath opened 13 years ago

mhemesath commented 13 years ago

Tobi's performance really goes down as the page size increases. Some of my tests run on very large forms, and 30+ seconds can pass between requests triggered by Tobi.

The issue probably lies somewhere in JSDOM, or JQuery in tandem with JSDOM. As I get time I'll try to find out the exact bottleneck and ways to reduce it.

tj commented 13 years ago

yeah i haven't profiled myself, but I've heard bad things about JSDOM's perf, not sure though

mhemesath commented 13 years ago

My tests seem to stress the most on Browser.fill. I'm wondering if we should consider taking Browser.locate and optimize it a bit. We can do that by attempting to retrieve the elements using the faster locators first, then exiting with the elements if result comes back.

Looking at the code below.. rather than running filter, maybe do each of the queries below until we detect elements. I don't think the assumption is too far off that people won't be using locators with the intent of them spanning multiple attributes?

Browser.prototype.locate = function(selector, locator){
  var self = this
    , $ = this.jQuery;

  // pseudo code. Try fastest searches first until something is found
  var elems = $(#locator)
  elems = elems.length < 1 ?  $("*[value=locator]") : elems;
  elems = elems.length < 1 ?  $("*[name=locator]") : elems;
  elems = elems.length < 1 ?  $("*").contains(locator) : elems;

  if (elems && !elems.length) throw new Error('failed to locate "' + locator + '" in context of selector "' + selector + '"');
  return elems;
};
tj commented 13 years ago

we should definitely profile first before considering anything, but yeah if there are obvious tweaks then I'm down, I dont even remember how I have things implemented

mhemesath commented 13 years ago

Alright.. I have GOT to resolve this as this is driving me nutz! I'm feeling ambitious.. gonna try swapping JSDOM for PhantomJS. If this fails, I'm gonna profile the $hit out of this project :p

tj commented 13 years ago

definitely worth profiling first to see if it's the html parser, jsdom, etc

mhemesath commented 13 years ago

Yeah, didn't get too far on swapping out the DOM impl. It will be tougher than I thought. Any ideas on a good profiling solution?

tj commented 13 years ago

v8 has a built in profiler, though I've never personally been able to get overly useful output from it, might have been doing something wrong, I think node-inspector (or something by the same guy) has profiling which I assume uses the same output

mhemesath commented 13 years ago

So I did a bit of profiling around a fill method which was taking about 5 seconds to complete. It appears that

jquery.Fn.extend.find

which is called from Browser.locate is slow. This is from the call to

Sizzle.uniqueSort

which triggers the jsDOM method

compareDocumentPosition

in jsdom/level3/core.js on line 81 which takes 2700ms to execute.

I know this is a crappy profile report but it just spit out a giant JSON object. I'm going to create a viewer of some sort to help make sense of this all, so we can make a better assessment. For now it appears that the problem lies in JSDOM.

tj commented 13 years ago

oh wow, interesting, definitely a hint in the right direction. I dont really see how a js implementation should be (much) slower than a browser, that seems pretty brutal considering a browser needs to do similar many times a second

mhemesath commented 13 years ago

Tobi doesn't override JSDOM's { QuerySelector : false } default value. I wonder what effect, if any, this would have. This may entirely change Sizzle's performance on JSDOM. Is there a reason QuerySelector was never enabled?

tj commented 13 years ago

no clue what that is

mhemesath commented 13 years ago

Me neither, all it says is:

QuerySelector default : false allowed : true

This feature is backed by sizzle but currently causes problems with some libraries. Enable this if you want document.querySelector and friends, but be aware that many libraries feature detect for this, and it may cause you a bit of trouble.

mhemesath commented 13 years ago

Drrr... stupid post. That would make JSDOM support document.querySelector. So basically, jQuery would be using sizzle packaged with JSDOM rather than sizzle packaged in itself.

tj commented 13 years ago

hmm, and the jsdom one is tweaked then im assuming? still seems really bizarre to be so slow, almost like you would have to try and make it that slow

mhemesath commented 13 years ago

I don't know if the jsdom sizzle is tweaked. It may make sense using that one anyway in case it is. I'm not too surprised by the performance. My forms are fairly large, and from experience, jQuery selector performance can degrade rather quickly if the DOM size grows large and the selector isn't optimized. I'll keep investigating.

masylum commented 13 years ago

I'm having performance issues with Browser.fill, I will try to optimize it a little bit.

mhemesath commented 13 years ago

JSDOM's latest release says it patched a main memory leaks. This may have been the culprit. https://gist.github.com/1129679