Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

increase perf for large client-side test suites #525

Closed
tj opened this Issue · 14 comments

2 participants

@paulmillr

As a side note, all tests run super slow in Safari 6 with shown web-inspector. And they're fast with hidden one.

Very slow test example: test (~2.5s).

@tj
Owner
tj commented

for that single case? wtf. Hard to say how mocha would have anything to do with that if it's within the function call, we'll have to profile

@paulmillr

Yep. new TestView container: testbed does a lot of stuff, but obviously it shouldn't take that long.

@tj
Owner
tj commented

that's probably a good sign, because it's probably something simple / very wrong haha

@paulmillr

Profiling of just one test

Seems like globals checker sucks in perf. Could you please point where can I disable it in index.html?

edit 1: Okay, disabled it already, same stuff. Probably it's expect.js problem.

edit 2: I can confirm that with Chai it runs in less then 1ms.

edit 3: Meh, sometimes it's very slow with chai too.

edit 4 So, the real problem seems to be the function

Assertion.prototype.assert = function (truth, msg, error) {
  var msg = this.flags.not ? error : msg
    , ok = this.flags.not ? !truth : truth;

  if (!ok) {
    throw new Error(msg);
  }

  this.and = new Assertion(this.obj);
};

If you compare DOM objects with it, it becomes super fucking slow.

@paulmillr paulmillr referenced this issue in Automattic/expect.js
Closed

Performance can be improved by 1000-fold #30

@tj
Owner
tj commented

interesting :s

@paulmillr

which leads us to slow i() (inspect) function.

One-line patch (expect.js, L520):

    if (typeof window !== 'undefined' &&
        typeof window.Element !== 'undefined' &&
        obj instanceof window.Element) {
          return obj.innerHTML;
    }

what should I improve before sending pull requests to expect.js / should.js / chai?

Edit: I think this issue can be closed now.

@tj
Owner
tj commented

hmm im confused why is there element serialization so often? in an .equal() call or sth?

@paulmillr

I think it's because DOM is complex, it does many recursive calls to parentNode, firstChild etc. etc. etc.

@tj
Owner
tj commented

i dont get what the serialization is even used for though

@tj
Owner
tj commented

oh hahahaha I see, pre-fab'ing the the error messages, gotcha, yeah I can see how that would be a big hit

@tj
Owner
tj commented

in that case it would be better to have this.assert take callbacks and defer the messages there

@paulmillr

Sure, but the DOM check is still needed.

@tj
Owner
tj commented

yup definitely, we can speed things up even more with the closure. I should probably do this for should.js too, I've never noticed it on the server end though

@tj tj closed this
@npmcomponent npmcomponent referenced this issue from a commit in npmcomponent/LearnBoost-expect.js
@paulmillr paulmillr Add inspect() case for DOM elements.
Serializing was VERY slow before the patch.

mochajs/mocha#525
925f6cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.