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(meta_generator): drop cheerio #3671

Merged
merged 7 commits into from Aug 21, 2019

Conversation

@curbengh
Copy link
Contributor

curbengh commented Aug 16, 2019

What does it do?

Less breaking alternative to #3669
x-post from that PR:

From the benchmark performed by @SukkaW , we can see cheerio—used by meta_generator filter—incurs significant perf cost. The feature was initially introduced by #3129.

Despite effort to improve the situation (#3667), the improvement was only minuscule. Hence, we try to avoid cheerio if possible.

How to test

git clone -b metagen-cheerio https://github.com/curbengh/hexo.git
cd hexo
npm install
npm test

Pull request tasks

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

This comment has been minimized.

Copy link

coveralls commented Aug 16, 2019

Coverage Status

Coverage decreased (-0.002%) to 97.152% when pulling fb42a5b on curbengh:metagen-cheerio into 49c7d4a on hexojs:master.

@stevenjoezhang

This comment has been minimized.

Copy link
Contributor

stevenjoezhang commented Aug 16, 2019

Maybe it's not necessary to use the xhml syntax?

<meta name="generator" content="Hexo %s" />

<meta name="generator" content="Hexo %s">

@curbengh curbengh force-pushed the curbengh:metagen-cheerio branch from b5c3866 to 2a4fc33 Aug 16, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Aug 16, 2019

btw, if anyone's worry about html in a code block, the brackets are encoded,

<figure class="highlight html"><table><tbody><tr><td class="code"><pre><span class="line"><span class="tag">&lt;<span class="name">head</span>&gt;</span><span class="tag">&lt;/<span class="name">head</span>&gt;</span></span><br></pre></td></tr></tbody></table></figure>
SukkaW added a commit to theme-suka/performance-test that referenced this pull request Aug 16, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Aug 17, 2019

I forgot to add a unit test where <head> already have meta[generator]. Maybe that's why coveralls decreased.
I'll add it.

Edit: added

decreased coverage may be due to shorter lines/functions.

@curbengh curbengh requested a review from SukkaW Aug 17, 2019
@curbengh curbengh mentioned this pull request Aug 17, 2019
1 of 2 tasks complete
Copy link
Member

SukkaW left a comment

LGTM

@curbengh curbengh force-pushed the curbengh:metagen-cheerio branch from caa5aaa to fb42a5b Aug 18, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Aug 18, 2019

squashed some commits.

@SukkaW SukkaW added this to the v4.0.0 milestone Aug 18, 2019
@SukkaW SukkaW self-requested a review Aug 18, 2019
@SukkaW
SukkaW approved these changes Aug 18, 2019
@curbengh curbengh mentioned this pull request Aug 18, 2019
3 of 4 tasks complete
@curbengh curbengh changed the title refactor(meta_generator): drop cheerio perf(meta_generator): drop cheerio Aug 18, 2019
@tomap tomap merged commit 651f34b into hexojs:master Aug 21, 2019
3 of 4 checks passed
3 of 4 checks passed
coverage/coveralls Coverage decreased (-0.002%) to 97.152%
Details
codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@curbengh curbengh deleted the curbengh:metagen-cheerio branch Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.