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(post): cache tags getter #5145

Merged
merged 2 commits into from Oct 21, 2023
Merged

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Jan 12, 2023

What does it do?

The PR tries to resolve #5119 (comment)

cc @stevenjoezhang

Screenshots

Flamegraph before the PR

image

Flamegraph after the PR

image

Pull request tasks

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

@github-actions
Copy link

How to test

git clone -b perf-list-tag-helper https://github.com/SukkaW/hexo.git
cd hexo
npm install
npm test

@coveralls
Copy link

coveralls commented Jan 12, 2023

Coverage Status

coverage: 99.53% (+0.001%) from 99.529% when pulling 50b4580 on SukkaW:perf-list-tag-helper into 7b588e7 on hexojs:v7.0.0.

@SukkaW
Copy link
Member Author

SukkaW commented Jan 16, 2023

@stevenjoezhang So it turns out the post.tags field is accessed more than once during rendering. Caching the getter could speed up the second access.

IMHO we could dig more into this, to see if we can prevent executing some functions over and over again.

Copy link
Member

@renbaoshuo renbaoshuo left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@renbaoshuo renbaoshuo added this to the 7.0.0 milestone Mar 6, 2023
@yoshinorin yoshinorin mentioned this pull request Mar 9, 2023
6 tasks
@stevenjoezhang stevenjoezhang changed the base branch from master to v7.0.0 July 9, 2023 17:37
@yoshinorin yoshinorin mentioned this pull request Jul 10, 2023
2 tasks
@SukkaW SukkaW merged commit 3059fa0 into hexojs:v7.0.0 Oct 21, 2023
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants