util: faster arrayToHash

[JacksonTian]

The util.format() is used frequently, make the method faster
is better.

[thefourtheye]

Why not a Set?

[JacksonTian]

The result of arrayToHash was used by downstream with hasOwnProperty(). If use Set, need change more lines.

[bnoordhuis]

LGTM but can you add a small benchmark in benchmark/misc and post before/after numbers?

[cjihrig]

LGTM

[lpinca]

I think that avoiding accessing the length property each time will speed up it a little more.

var l = array.length;
var val;
var i;

for (i = 0;  i < l; i++) {
  val = array[i];
  hash[val] = true;
}

[JacksonTian]

V8 can speedup it with OSR.

[tflanagan]

Why make it do more work?

[yorkie]

using let here might have a more clear scope definition?

[mscdex]

v8 is smart enough these days to cache the length automatically, so this shouldn't be an issue.

[JacksonTian]

Benchmark is here：

var util = require('util');

var common = require('../common.js');

var bench = common.createBenchmark(main, {n: [5e6]});

function main(conf) {
  var n = conf.n | 0;

  bench.start();
  for (var i = 0; i < n; i += 1) {
    var r = util.inspect({a: 'a', b: 'b', c: 'c', d: 'd'});
  }
  bench.end(n);
}

before:
util/inspect.js n=5000000: 45599.83340

after:
util/inspect.js n=5000000: 63209.71860

cc @bnoordhuis

[mscdex]

LGTM

[bnoordhuis]

Thanks, LGTM. CI: https://ci.nodejs.org/job/node-test-pull-request/810/

[bnoordhuis]

Linking to nodejs/build#263.

[evanlucas]

Trying CI again https://ci.nodejs.org/job/node-test-pull-request/887/

[JacksonTian]

What's wrong with CI?

[cjihrig]

Running CI again, just because it's been 2 weeks: https://ci.nodejs.org/job/node-test-pull-request/991/

[trevnorris]

Anything holding this up?

[cjihrig]

I don't think so. Running CI again, then hopefully this can land. https://ci.nodejs.org/job/node-test-pull-request/1106/

[jasnell]

LGTM

[jasnell]

Will get this landed.

[jasnell]

Landed in 3e740ca

[jbergstroem]

Minor heads up, typo in commit message: R-URL -> PR-URL. The grammar could have been slightly improved as well -- I would have liked reading what kind of speed improvement it brought.

[jasnell]

Ugh. OK. Thanks for the heads up
On Dec 30, 2015 11:36 PM, "Johan Bergström" notifications@github.com
wrote:

Minor heads up, typo in commit message: R-URL -> PR-URL. The grammar
could have been slightly improved as well -- I would have liked reading
what kind of speed improvement it brought.

—
Reply to this email directly or view it on GitHub
#3964 (comment).