Skip to content

perf(utils): improve getProp(), setProp(), delProp(), setGetter() performance#68

Merged
SukkaW merged 2 commits intohexojs:masterfrom
dailyrandomphoto:improve-extractPropKey
Dec 24, 2019
Merged

perf(utils): improve getProp(), setProp(), delProp(), setGetter() performance#68
SukkaW merged 2 commits intohexojs:masterfrom
dailyrandomphoto:improve-extractPropKey

Conversation

@dailyrandomphoto
Copy link
Member

@dailyrandomphoto dailyrandomphoto commented Dec 17, 2019

I found that on a huge site, the execution time of String.split() / extractPropKey() is very long.

function extractPropKey(key) {
  return key.split('.');
}

(https://github.com/hexojs/warehouse/blob/master/lib/util.js#L22-L24)

A benchmark using a hexo dummy website (with 300 posts): (clone https://github.com/SukkaLab/hexo-many-posts.git)

  • extractPropKey() has been called for 7,201,086 times.

  • For input values, they are all simple keys without "."

3325949  tag_id
2474477  post_id
 894848  _id
 178803  name
 131535  category_id
  17070  path
  16950  permalink
  16936  slug
  14418  length
  12964  date
  12490  posts
   9722  source
   7588  modified
   7468  hash
   6580  parent
   5084  published
   4460  updated
   4460  title
   4460  raw
   4460  layout
   4460  full_source
   4460  comments
   4460  _content
   4446  tags
   4446  photos
   4446  link
   4446  categories
   4446  asset_dir
   3820  more
   3820  excerpt
   3820  content
   3174  id
    120  renderable

A benchmark using a hexo dummy website (with 1500 posts): (clone https://github.com/SukkaLab/hexo-many-posts.git 5 times)

  • Execution Time of extractPropKey(): 19.78s

2019121701

Solution

This PR skips the call to split() and for-loops, handle simple keys directly.

@dailyrandomphoto dailyrandomphoto changed the title perf(utils): don't call String.split() for simple keys perf(utils): improve getProp(), setProp(), delProp(), setGetter() performance Dec 17, 2019
SukkaW
SukkaW requested changes Dec 17, 2019
View reviewed changes
SukkaW
SukkaW reviewed Dec 17, 2019
View reviewed changes
Copy link
Member

@SukkaW SukkaW left a comment

Choose a reason for hiding this comment

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

Has it been tested in a dummy site?

@dailyrandomphoto
Copy link
Member Author

dailyrandomphoto commented Dec 17, 2019

@SukkaW
Here is data from test/benchmark.sh ran on Travis. (master, this pr)

A benchmark using a hexo dummy website (with 300 posts):

Node.js 8

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
Current Master Branch 24.228s 0.611s 584.676MB 15.823s 0.667s 767.297MB
This PR 21.134s 0.562s 578.789MB 13.698s 0.557s 753.648MB

Node.js 10

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
Current Master Branch 25.258s 0.780s 589.504MB 15.836s 0.777s 789.211MB
This PR 18.150s 0.576s 609.074MB 11.686s 0.591s 746.203MB

Node.js 12

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
Current Master Branch 24.923s 0.784s 573.012MB 14.703s 0.783s 808.902MB
This PR 19.895s 0.596s 547.336MB 11.947s 0.712s 807.457MB

Node.js 13

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
Current Master Branch 25.070s 0.707s 589.926MB 15.299s 0.716s 807.445MB
This PR 22.133s 0.620s 584.203MB 13.568s 0.717s 824.426MB

@SukkaW SukkaW merged commit 1529483 into hexojs:master Dec 24, 2019
@SukkaW SukkaW mentioned this pull request Jan 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SukkaW SukkaW SukkaW approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dailyrandomphoto @SukkaW