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

perf(external_link): drop cheerio and use regex #3685

Merged
merged 6 commits into from Aug 26, 2019

Conversation

@SukkaW
Copy link
Member

SukkaW commented Aug 22, 2019

What does it do?

Drop cheerio in external_link() helper for better performance.

How to test

git clone -b drop-cheerio-for-external-link https://github.com/sukkaw/hexo.git
cd hexo
npm install
npm test

Screenshots

NodeJS 8 NodeJS 10 NodeJS 12
cheerio@0.22 (hexojs/hexo#master) 7.64 6.62 7.54
regex (This PR) 7.60 6.57 7.40
NodeJS 8 NodeJS 10 NodeJS 12
cheerio@0.22 (hexojs/hexo#master) 6.55 5.53 6.15
regex (This PR) 6.48 5.42 6.10
NodeJS 8 NodeJS 10 NodeJS 12
cheerio@0.22 (hexojs/hexo#master) 6.12 5.92 6.57
regex (This PR) 6.03 5.88 6.44

The performance increased by nearly 1%.

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 22, 2019

Coverage Status

Coverage decreased (-0.003%) to 97.152% when pulling 07d83c7 on SukkaW:drop-cheerio-for-external-link into 6f16116 on hexojs:master.

@SukkaW SukkaW requested review from segayuu and curbengh Aug 22, 2019
@curbengh curbengh referenced this pull request Aug 22, 2019
3 of 4 tasks complete
Copy link
Contributor

segayuu left a comment

LGTM!

@SukkaW

This comment has been minimized.

Copy link
Member Author

SukkaW commented Aug 22, 2019

@segayuu I am thinking is it worthy.

In this PR I drop cheerio, completely rewrite this part use regex and replace(), but only result in less than 1% performance increases.

@curbengh

This comment has been minimized.

Copy link
Contributor

curbengh commented Aug 23, 2019

This PR drops cheerio (along with its deps) and replace it with even less LOC than before. The way I see it, this is equivalent to >99% reduction of LOC. In this case, I'm certain regex+replace can perform at least as fast as cheerio.

LOC reduction + no loss in performance = 💯% worth it.

@SukkaW SukkaW requested review from curbengh and segayuu Aug 23, 2019
@curbengh curbengh added this to the v4.0.0 milestone Aug 24, 2019
@curbengh curbengh merged commit f3e0f5c into hexojs:master Aug 26, 2019
2 of 5 checks passed
2 of 5 checks passed
codeclimate 1 issue to fix
Details
coverage/coveralls Coverage decreased (-0.003%) to 97.152%
Details
Travis CI - Pull Request Build Created
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.