Skip to content

Conversation

@gxcsoccer
Copy link
Member

调整一下 ByteBuffer.get 参数判断的顺序

@gxcsoccer gxcsoccer requested review from dead-horse, fengmk2 and pmq20 May 10, 2017 18:36
@mention-bot
Copy link

@gxcsoccer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fengmk2, @dead-horse and @pmq20 to be potential reviewers.

@fengmk2
Copy link
Member

fengmk2 commented May 10, 2017

前后 benchmark 对比呢?

};

ByteBuffer.prototype.get = function (index, length) {
if (index instanceof Buffer || Array.isArray(index)) { // get (byte[] dst, int offset, int length)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个判断比较耗时,把它挪到后面去,在 hessian.js 里大部分情况是 get() 或者 get(index)

@gxcsoccer
Copy link
Member Author

  node version: v7.10.0, date: Thu May 11 2017 02:58:46 GMT+0800 (CST)
  Starting...
  4 tests completed.

  old get()      x  2,234,273 ops/sec ±1.45% (90 runs sampled)
  new get()      x 24,872,342 ops/sec ±2.13% (89 runs sampled)
  old get(index) x  2,339,452 ops/sec ±0.81% (93 runs sampled)
  new get(index) x 25,533,534 ops/sec ±0.84% (100 runs sampled)

@gxcsoccer
Copy link
Member Author

ping @fengmk2

lib/byte.js Outdated
this.checkArraySize(dst.length, offset, length);
this.checkForUnderflow(length);

if (is.buffer(dst)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接用 Buffer.isBuffer() 吧,就不需要引入 is-type-of

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 改掉这个

// old get() x 2,234,273 ops/sec ±1.45% (90 runs sampled)
// new get() x 24,872,342 ops/sec ±2.13% (89 runs sampled)
// old get(index) x 2,339,452 ops/sec ±0.81% (93 runs sampled)
// new get(index) x 25,533,534 ops/sec ±0.84% (100 runs sampled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这性能优化有点吊。。。一个数量级。

@fengmk2
Copy link
Member

fengmk2 commented May 11, 2017

@gxcsoccer 去掉 is-type-of 的依赖,你自己合并发布新版本。

@pmq20
Copy link
Contributor

pmq20 commented May 11, 2017

👏 赞 没想到 index instanceof Buffer || Array.isArray(index)这么耗时

@gxcsoccer gxcsoccer merged commit 9f490d5 into master May 11, 2017
@gxcsoccer gxcsoccer deleted the enhance-pref branch May 11, 2017 06:54
@gxcsoccer
Copy link
Member Author

@fengmk2 加个权限

@dead-horse
Copy link
Member

@gxcsoccer done

@gxcsoccer
Copy link
Member Author

+ [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants