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

test: parallel/test-stringbytes-external is too slow #2370

Closed
bnoordhuis opened this issue Aug 13, 2015 · 10 comments
Closed

test: parallel/test-stringbytes-external is too slow #2370

bnoordhuis opened this issue Aug 13, 2015 · 10 comments
Assignees
Labels
arm Issues and PRs related to the ARM platform. test Issues and PRs related to the tests.

Comments

@bnoordhuis
Copy link
Member

$ time iojs test/parallel/test-stringbytes-external.js

real   0m3.887s
user   0m3.769s
sys    0m0.157s

It fails sporadically on the rpi1 buildbot: https://jenkins-iojs.nodesource.com/job/node-test-commit-arm/153/nodes=pi1-raspbian-wheezy/tapTestReport/test.tap-667/

/cc @trevnorris

@bnoordhuis bnoordhuis added the test Issues and PRs related to the tests. label Aug 13, 2015
@brendanashworth brendanashworth added the arm Issues and PRs related to the ARM platform. label Aug 13, 2015
@trevnorris trevnorris self-assigned this Aug 13, 2015
@trevnorris
Copy link
Contributor

Major issue identified. All the string concatenation in the assert.*() calls in the for loops is killing things. e.g. change

// make sure Buffers from externals are the same
for (var i = 0; i < c_bin.length; i++) {
  assert.equal(c_bin[i], c_ucs[i], c_bin[i] + ' == ' + c_ucs[i] +
               ' : index ' + i);
}

to:

// make sure Buffers from externals are the same
for (var i = 0; i < c_bin.length; i++) {
  if (c_bin[i] !== c_ucs[i])
  assert.equal(c_bin[i], c_ucs[i]);
}

a good chunk of time is lost. Same with the other 4 for loops below.

@bnoordhuis Easy fix is to just remove all the string concatenation. Thoughts?

@bnoordhuis
Copy link
Member Author

Seems reasonable to me.

@evanlucas
Copy link
Contributor

Yea, I got it down quite a bit by changing those asserts

[~/dev/code/forks/io.js]
:] ➜ $ time ./iojs test/parallel/test-stringbytes-external.js                                                                                                                     (master) 
        0.36 real         0.33 user         0.03 sys

@trevnorris
Copy link
Contributor

Ref: #2410

@Fishrock123
Copy link
Contributor

Should be fixed in 37ee43e

@rvagg
Copy link
Member

rvagg commented Sep 22, 2015

going to reopen this, it's failing still/again, e.g. https://ci.nodejs.org/job/node-test-binary-arm/RUN_SUBSET=2,nodes=pi1-raspbian-wheezy/34

not ok 98 test-stringbytes-external.js
  ---
  duration_ms: 208.602
  ...

@Trott
Copy link
Member

Trott commented Oct 8, 2015

Hopefully #3287 fixes this.

@Trott
Copy link
Member

Trott commented Oct 8, 2015

@Trott
Copy link
Member

Trott commented Feb 3, 2016

I'm pretty sure this is fixed at this point. Let's try a stress test: https://ci.nodejs.org/job/node-stress-single-test/398/nodes=pi1-raspbian-wheezy/console

@bnoordhuis
Copy link
Member Author

The buildbot went offline halfway through but 323 successful and 0 failing runs seem like good indication this issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm Issues and PRs related to the ARM platform. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

7 participants