FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[WIP] refactor: enhance decode performance by gxcsoccer · Pull Request #74 · node-modules/hessian.js · GitHub

/ hessian.js Public

[WIP] refactor: enhance decode performance - #74

Open
gxcsoccer wants to merge 1 commit into
masterfrom
enhance-perf
ConversationCommits (1)ChecksFiles changed
Open

[WIP] refactor: enhance decode performance#74
gxcsoccer wants to merge 1 commit into
masterfrom
enhance-perf

Conversation

gxcsoccer commented May 10, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

初步优化了下 number, date 和 string，性能和孝达的 fast hessian2 差不多了

  Fast hessian2 decode: number x 10,408,116 ops/sec ±1.10% (87 runs sampled)
  Fast hessian2 decode: date   x  4,139,701 ops/sec ±1.04% (88 runs sampled)
  Fast hessian2 decode: string x  1,843,122 ops/sec ±5.17% (61 runs sampled)

  js hessian2 decode: number x 11,456,920 ops/sec ±1.18% (88 runs sampled)
  js hessian2 decode: date   x  3,964,458 ops/sec ±1.14% (90 runs sampled)
  js hessian2 decode: string x  1,617,342 ops/sec ±1.08% (93 runs sampled)

gxcsoccer requested review from dead-horse and fengmk2 May 10, 2017 18:40

mention-bot commented May 10, 2017

Copy link
Copy Markdown

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

gxcsoccer commented May 10, 2017
View reviewed changes
Comment thread lib/v1/decoder.js
* @api public
*/
proto.readNull = function () {
this._checkLabel('readNull', 'N');

gxcsoccer May 10, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

如果把 readNull，readInt 等这种 api 都变成私有的，只暴露 read，那 checkLabel 这个是没必要的

gxcsoccer commented May 10, 2017
View reviewed changes
Comment thread lib/v2/decoder.js
var code = this.byteBuffer.get();
if (code === 0x4a) {
return new Date(utils.handleLong(this.byteBuffer.getLong()));
return new Date(this.byteBuffer.getLong().toNumber());

gxcsoccer May 10, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

针对 date，直接 toNumber 就好了

dead-horse reviewed May 11, 2017
View reviewed changes
Comment thread lib/v1/decoder.js
head = this.byteBuffer.get();
l = utils.lengthOfUTF8(head);
this.byteBuffer.skip(l - 1);
ch = this.byteBuffer.get();

dead-horse May 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

这个改完会有多大性能提升？

gxcsoccer May 11, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

少了一次循环，我之前有测，==

gxcsoccer May 11, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

优化前

hessian2 decode: string x 1,251,833 ops/sec ±1.35% (86 runs sampled)

优化后

hessian2 decode: string x 1,482,684 ops/sec ±1.29% (85 runs sampled)

pmq20 commented May 11, 2017 •
edited
Loading

Copy link
Copy Markdown

初步优化了下 number, date 和 string，性能和孝达的 fast hessian2 差不多了

这俩组数据要在同一台机器 && Node.js version 上跑才有意义。我试一下我的机器 && node 8-pre

pmq20 commented May 11, 2017

Copy link
Copy Markdown

@gxcsoccer 同一台机器，同样的 node 版本下：

  node version: v8.0.0-pre

  hessian2 decode: number x 4,721,288 ops/sec ±0.54% (87 runs sampled)
  hessian2 decode: date   x 2,599,811 ops/sec ±0.61% (92 runs sampled)
  hessian2 decode: string x   557,185 ops/sec ±1.01% (88 runs sampled)

  Fast hessian2 decode: number x 9,706,914 ops/sec ±1.41% (84 runs sampled)
  Fast hessian2 decode: date   x 3,861,312 ops/sec ±1.18% (86 runs sampled)
  Fast hessian2 decode: string x 1,671,423 ops/sec ±5.69% (60 runs sampled)

fengmk2 reviewed May 11, 2017
View reviewed changes
Comment thread lib/v1/decoder.js
*/
proto.init = function (buf) {
this.byteBuffer = ByteBuffer.wrap(buf);
this.byteBuffer._bytes = buf;

fengmk2 May 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

为何要使用这种私有 api？

fengmk2 May 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this.byteBuffer.reset(buf) 这种不是更好？

gxcsoccer May 11, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

reset 没有入参，这里是 WIP，还不是最终版本，先找到优化点，后面可以针对性的对 api 做调整

ByteBuffer.prototype.reset = function () {
  this._offset = 0;
};

fengmk2 commented May 11, 2017

Copy link
Copy Markdown
Member

@gxcsoccer 比之前的 js 版本优化多少？加上对比

fengmk2 commented May 11, 2017

Copy link
Copy Markdown
Member

ci 加上 node 7

codecov Bot commented May 11, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #74 into master will decrease coverage by 0.57%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
- Coverage   96.09%   95.51%   -0.58%     
==========================================
  Files           7        7              
  Lines        1076     1093      +17     
  Branches      202      204       +2     
==========================================
+ Hits         1034     1044      +10     
- Misses         42       49       +7
Impacted Files Coverage Δ
index.js 100% <100%> (ø) ⬆️
lib/v1/decoder.js 98.33% <100%> (+0.07%) ⬆️
lib/v2/decoder.js 92.52% <100%> (+0.13%) ⬆️
lib/utils.js 83.33% <0%> (-16.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bea205...f8ad41a. Read the comment docs.

gxcsoccer commented May 11, 2017

Copy link
Copy Markdown
Member Author

@pmq20 你跑的时候 byte 还没有合并，现在再跑跑试试

gxcsoccer commented May 11, 2017

Copy link
Copy Markdown
Member Author

优化前

  Hessian Decode Benchmark
  node version: v7.10.0, date: Thu May 11 2017 16:53:41 GMT+0800 (CST)
  Starting...
  3 tests completed.

  hessian2 decode: number x 1,647,104 ops/sec ±1.51% (83 runs sampled)
  hessian2 decode: date   x   412,326 ops/sec ±2.43% (81 runs sampled)
  hessian2 decode: string x   169,817 ops/sec ±1.93% (84 runs sampled)

优化后

  Hessian Decode Benchmark
  node version: v7.10.0, date: Thu May 11 2017 16:54:07 GMT+0800 (CST)
  Starting...
  3 tests completed.

  hessian2 decode: number x 9,965,533 ops/sec ±2.19% (85 runs sampled)
  hessian2 decode: date   x 3,342,223 ops/sec ±2.62% (81 runs sampled)
  hessian2 decode: string x 1,272,044 ops/sec ±1.73% (84 runs sampled)

gxcsoccer commented May 11, 2017

Copy link
Copy Markdown
Member Author

number 和 string 的优化主要来自于 byte.get node-modules/byte#25

pmq20 commented May 11, 2017

Copy link
Copy Markdown 
  hessian2 decode: number x 9,840,410 ops/sec ±1.10% (92 runs sampled)
  hessian2 decode: date   x 3,965,427 ops/sec ±0.47% (90 runs sampled)
  hessian2 decode: string x 1,473,963 ops/sec ±0.72% (91 runs sampled)

  Fast hessian2 decode: number x 10,883,935 ops/sec ±1.01% (91 runs sampled)
  Fast hessian2 decode: date   x  4,356,561 ops/sec ±0.88% (90 runs sampled)
  Fast hessian2 decode: string x  1,886,761 ops/sec ±4.78% (62 runs sampled)

pmq20 commented May 11, 2017

Copy link
Copy Markdown

💯 确实性能和 fast hessian2 差不多了

fengmk2 commented Aug 2, 2017

Copy link
Copy Markdown
Member

@gxcsoccer 这周忙完继续搞这个，最近 rpc 比较多压力。

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

fengmk2 fengmk2 left review comments
dead-horse dead-horse left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL