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

fix(moment.locale): avoid lookup repeatedly with the wrong names. #4007

Merged
merged 1 commit into from
Dec 24, 2019

Conversation

dailyrandomphoto
Copy link
Member

@dailyrandomphoto dailyrandomphoto commented Dec 24, 2019

What does it do?

Hexo language code is different with Moment.js.
If we set language like this,

# _config.yml
language:
timezone:

or

# _config.yml
language: zh-CN
timezone:

then default or zh-CN will pass to moment.locale(lang).

const lang = this.page.lang || this.page.language || config.language;
...
if (lang) date = date.locale(lang); // lang = 'default' or 'zh-CN'

Whenever .locale (lang) is called, it causes moment to find the locale file repeatedly.
2019122402

This PR adds toMomentLocale() to convert lang code, avoid lookup repeatedly with the wrong names.

How to test

git clone -b fix-moment-locale https://github.com/dailyrandomphoto/hexo.git
cd hexo
npm install
npm test

Pull request tasks

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

@coveralls
Copy link

coveralls commented Dec 24, 2019

Coverage Status

Coverage increased (+0.005%) to 97.082% when pulling 531780f on dailyrandomphoto:fix-moment-locale into 92c705d on hexojs:master.

@dailyrandomphoto
Copy link
Member Author

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

Measure 5 times and calculate the average for each benchmark.

Node.js 8

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master: lang = en 61.816s 1.997s 1060.584MB 43.517s 1.828s 1109.915MB
master: lang = zh-CN 60.432s 1.902s 1046.009MB 42.284s 1.763s 1102.388MB
this PR: lang = zh-CN 60.407s 1.957s 1049.878MB 42.187s 1.794s 1103.745MB

Node.js 10

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master: lang = en 50.663s 1.814s 1074.596MB 36.368s 1.709s 1104.663MB
master: lang = zh-CN 52.358s 1.832s 1072.98MB 37.554s 1.74s 1106.869MB
this PR: lang = zh-CN 50.324s 1.805s 1073.225MB 36.904s 1.707s 1111.981MB

Node.js 12

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master: lang = en 52.957s 1.937s 1076.198MB 38.374s 1.87s 1101.208MB
master: lang = zh-CN 54.357s 1.927s 1102.307MB 38.912s 1.826s 1111.927MB
this PR: lang = zh-CN 50.377s 1.874s 1077.271MB 36.067s 1.755s 1110.664MB

Node.js 13

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master: lang = en 50.829s 1.815s 1081.803MB 37.32s 1.718s 1100.031MB
master: lang = zh-CN 52.473s 1.792s 1070.279MB 37.974s 1.745s 1097.203MB
this PR: lang = zh-CN 51.496s 1.805s 1075.648MB 36.472s 1.699s 1100.096MB

@@ -23,7 +23,7 @@ SchemaTypeMoment.prototype.cast = function(value, data) {
const { options } = this;
value = toMoment(value);

if (options.language) value = value.locale(options.language);
if (options.language) value = value.locale(toMomentLocale(options.language));
Copy link
Member Author

Choose a reason for hiding this comment

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

options.language always is undefined.
This is a bug, or this is unnecessary code.

Copy link
Member

@SukkaW SukkaW Dec 24, 2019

Choose a reason for hiding this comment

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

As far as I know, models/types/moment is only used in Post and Page model.

type: Moment,

type: Moment,

What happened then is still a mystery. Only tommy351 knows.

lib/plugins/helper/date.js Outdated Show resolved Hide resolved
@SukkaW SukkaW merged commit 75fea56 into hexojs:master Dec 24, 2019
@dailyrandomphoto
Copy link
Member Author

Benchmark for relative_link: true

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

Measure 5 times and calculate the average for each benchmark.

Benchmark conditions:

Node.js 8

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master 148.839s 2.38s 1102.397MB 125.162s 2.111s 1181.39MB
this PR 81.695s 2.219s 1112.943MB 63.956s 1.973s 1172.119MB

Node.js 10

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master 121.885s 2.045s 1203.274MB 106.81s 1.859s 1242.954MB
this PR 68.21s 2.045s 1167.641MB 53.912s 1.888s 1206.12MB

Node.js 12

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master 127.652s 2.065s 1128.723MB 114.08s 1.953s 1210.77MB
this PR 69.073s 2.112s 1141.249MB 54.851s 1.945s 1183.367MB

Node.js 13

Total time (Cold) Save DB (Cold) Memory (Cold) Total time (Hot) Save DB (Hot) Memory (Hot)
master 125.619s 2s 1129.541MB 114.066s 1.898s 1181.748MB
this PR 69.611s 2.1s 1125.007MB 54.739s 1.915s 1158.469MB

thom4parisot pushed a commit to thom4parisot/hexo that referenced this pull request Jan 17, 2020
@SukkaW SukkaW mentioned this pull request Jul 25, 2020
22 tasks
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

3 participants