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

Updated prepare_date_terms to only call date_i18n once #2214

Merged
merged 10 commits into from
May 27, 2021

Conversation

Rahmon
Copy link
Contributor

@Rahmon Rahmon commented May 24, 2021

This is a "take over" of #1958 just to address some minor things.

Description of the Change

Calling date_i18n a lot gets pretty expensive. Only calling it once for each prepare_date_terms call is much more efficient and make indexing faster.

Props @WPprodigy
cc: @rinatkhaziev @nickdaugherty @pschoffer @netsuso @parkcityj

Alternate Designs

N/A

Benefits

Removes extraneous calls to date_i18n and makes indexing faster.

Possible Drawbacks

N/A

Verification Process

There are 4 calls to prepare_date_terms() per post being indexed. Each call resulted in 12 calls to date_i18n(), which ends up going down a bit of a rabbit hole and eventually adding up to a lot of time spent doing filters and alloption calls. So per page of 500 posts being indexed, that is 24,000 calls (500 x 12*4)- seen in cachegrind.

Before the changes, a test group of 500 posts took 7.91601 seconds running prepare_date(). After this PR, 0.85888 seconds.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Closes: #1958

Changelog Entry

Changed prepare_date_terms to only call date_i18n once. Props @WPprodigy

includes/classes/Indexable/Post/Post.php Outdated Show resolved Hide resolved
includes/classes/Indexable/Post/Post.php Show resolved Hide resolved
tests/php/indexables/TestPost.php Outdated Show resolved Hide resolved
tests/php/indexables/TestPost.php Outdated Show resolved Hide resolved
tests/php/indexables/TestPost.php Outdated Show resolved Hide resolved
@felipeelia felipeelia removed the qa label May 26, 2021
@felipeelia felipeelia assigned Rahmon and unassigned felipeelia May 26, 2021
@felipeelia
Copy link
Member

felipeelia commented May 26, 2021

@Rahmon I forgot to mention but there are some warnings in PHPCS that should be addressed as well. Thanks! actually, nevermind. They seem to come from the current codebase. Sorry.

@Rahmon Rahmon assigned felipeelia and unassigned Rahmon May 26, 2021
@felipeelia felipeelia merged commit c6a260e into develop May 27, 2021
@felipeelia felipeelia deleted the refactor/pr-1958 branch May 27, 2021 15:04
@felipeelia felipeelia added this to the 3.6.0 milestone Jul 5, 2021
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.

3 participants