Closed david-binda closed 6 years ago
kraftbj
commented
6 years ago Profiled a site against master, this PR, and one that uses WP_Query while update_post_(meta|term)_cache' => false.
For full sitemap generation, master took 30.9s, the PR took 15.6s, the WP_Query modified 20.4s. While not fully conclusive, it confirms @david-binda's results that this is substantially more performant.
pkevan
commented
6 years ago @kraftbj what were the stats for the site you tested on (for reference), i.e. number of posts, total sitemaps generated?
kraftbj
commented
6 years ago @pkevan 1,300 post posts generating 1059 msm_sitemap posts, running PHP 7.0.x. Minimal plugins (MSM + Debug Bar, Query Monitor, etc)
mattoperry
commented
6 years ago @david-binda @pkevan looks merge-able -- any objections?
david-binda
commented
6 years ago No objections here, @mattoperry
kraftbj
commented
6 years ago Ran it against a test site with ~60k post posts generating 6k sitemaps (random post dates from 1/1/2000 to today). This PR completed the task in half the time as master.
Merging.
mjangda
commented
6 years ago We need to make sure to flag the backwards compatibility issue that this change breaks (and, ideally, alert impacted clients, if we can).
kraftbj
commented
6 years ago @mjangda I don't grok the backwards-compat issue looking at the PR that introduced the comment. L478 and 479 ( https://github.com/Automattic/msm-sitemap/pull/129/files#diff-67d63994628dd99d09072eee489c7487R478 ) would prop up the Loop to satisfy the issue as I understand it.
Could you point me in the right direction to grok the breakage so I can ensure we document it accurately?
Testing the build on cold caches shows performance improvement of ~33%.
I have also been testing a situation in which the direct SQL query would return whole line which is then passed to
get_post(), but it does not show any performance improvement and shows performance decrease agains this version with warm caches (eg.: rebuild due to post edit situation).