Opened 2 years ago
Last modified 6 weeks ago
#61343 new enhancement
Improve caching in get_calendar function
| Reported by: | spacedmonkey | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Widgets | Version: | |
| Severity: | normal | Keywords: | good-first-bug has-patch reporter-feedback has-unit-tests 2nd-opinion |
| Cc: | Focuses: | performance |
Description
The get_calendar function is used to generate the calendar. There are a number of things that can be done to improve the performance of this function.
- Replace the raw query to posts table with call to WP_Query query class.
- Change cache invalidation from
add_action( 'save_post', 'delete_get_calendar_cache' ); add_action( 'delete_post', 'delete_get_calendar_cache' );
to
add_action( 'clean_post_cache', 'delete_get_calendar_cache' );
- Instead of store all caches in a simple cache key, break them up and invalidate caches in another way.
Attachments (1)
Change History (16)
This ticket was mentioned in PR #6708 on WordPress/wordpress-develop by @narenin.
2 years ago
#1
- Keywords has-patch added
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
2 years ago
This ticket was mentioned in Slack in #core-performance by adamsilverstein. View the logs.
19 months ago
This ticket was mentioned in PR #11547 on WordPress/wordpress-develop by @MythThrazz.
5 months ago
#8
- Keywords has-unit-tests added
## Summary
- Replace monolithic calendar cache (single array with all variations) with individual cache keys per variation — reduces deserialization overhead on reads
- Switch invalidation hooks from
save_post/delete_posttoclean_post_cache— avoids unnecessary flushes on autosaves, revisions, and draft saves - Use
wp_cache_flush_group('calendar')for invalidation with a generation counter fallback for object cache implementations lacking group flush support - Replace MySQL date arithmetic (
DATE_FORMAT/DATE_ADD) with PHP nativegmdate()— eliminates unnecessary DB roundtrip
## Technical Details
### Cache Architecture (before → after)
Before: All calendar variations stored in one serialized array under key get_calendar. Every read deserializes all months/post-types. Every write re-serializes the entire array. delete_get_calendar_cache() nukes everything via wp_cache_delete.
After: Each variation gets its own cache key (get_calendar_{hash}). Reads fetch only the requested entry. delete_get_calendar_cache() uses wp_cache_flush_group('calendar') to clear all entries.
### Persistent Cache Compatibility
Object cache implementations that don't support flush_group get a generation counter fallback: delete_get_calendar_cache() increments a generation value, and the cache key includes that generation — effectively invalidating all previous entries without needing to enumerate them.
### Raw SQL Queries Preserved
The ticket suggested replacing $wpdb queries with WP_Query, but SELECT DISTINCT DAYOFMONTH(post_date) returns max 31 rows while WP_Query would load all posts in the month. The existing PR #6708 makes this replacement and introduces a performance regression. We keep the efficient raw queries.
Supersedes: https://github.com/WordPress/wordpress-develop/pull/6708
## Test plan
- [x] All 10 existing + new tests pass (27 assertions)
- [x] PHPCS clean on all 3 modified files
- [x] Cache invalidation works via
delete_get_calendar_cache() - [x] Individual cache keys: different post types cached independently
- [x]
flush_groupclears all variations at once - [x] Backwards compatibility:
delete_get_calendar_cache()function signature unchanged - [ ] Verify with a persistent object cache plugin (Redis/Memcached)
---
AI assistance: Yes
Tool(s): Claude Code (Anthropic)
Model(s): Claude Opus 4.6
Used for: Performance analysis, identifying regression in existing PR #6708, implementation of cache restructure with generation counter fallback, and writing unit tests. All code was reviewed and tested.
---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
🤖 Generated with Claude Code
#9
@
5 months ago
New PR in #11547 with a different approach than PR #6708:
Cache architecture: Replaced the monolithic cache array (all variations in one key) with individual cache keys per variation (get_calendar_{hash}). Reads no longer deserialize all months/post-types, and writes only touch one entry.
Cache invalidation: Switched from save_post/delete_post to clean_post_cache as suggested. Uses wp_cache_flush_group('calendar') with a generation counter fallback for persistent cache implementations that don't support group flushing.
Raw SQL kept: The suggestion to replace $wpdb queries with WP_Query was evaluated but not implemented — SELECT DISTINCT DAYOFMONTH(post_date) returns max 31 rows, while WP_Query with posts_per_page=-1 (as in PR #6708) would load all posts in the month. The raw queries are more efficient here.
Date math: Replaced the MySQL DATE_FORMAT(DATE_ADD(...)) query with PHP gmdate() to eliminate a DB roundtrip for pure date calculation.
Includes unit tests for cache invalidation, individual keys, and flush_group behavior. All existing tests pass.
@MythThrazz commented on PR #11547:
3 months ago
#11
Refreshed against trunk and tightened the scope.
This stores one cache entry per calendar variation (keyed by args) instead of a single growing serialized array, and invalidates via wp_cache_flush_group() on the calendar group where supported, with a generation-counter fallback for object cache backends that don't support group flushing. Invalidation moves to the clean_post_cache hook, which covers both create and delete paths.
Two items addressed from review:
- Reverted an unrelated MySQL→PHP week-to-month date conversion that had crept in; this change is now strictly about caching.
- The invalidation test now fires the real
clean_post_cache()hook (rather than calling the cache-clear helper directly), exercising the default-filters wiring end to end.
Tests_General_GetCalendar passes (10 tests). Known low-impact follow-up: on backends without flush_group support, superseded per-generation keys rely on LRU eviction (never served stale); happy to add a TTL if preferred.
---
AI assistance: Yes
Tool(s): Claude Code (Anthropic)
Model(s): Claude Opus 4.8
Used for: implementation, independent review (cross-model), and tests; reviewed and verified by me.
This ticket was mentioned in Slack in #core by adrianduffell. View the logs.
2 months ago
#13
@
2 months ago
- Milestone 7.1 → Future Release
Moving this ticket to a future release, as it looks unlikely to be completed before the upcoming 7.1 beta.
#14
@
6 weeks ago
- Keywords 2nd-opinion added
Looking at the invalidation hook change in both PRs (save_post/delete_post → clean_post_cache), I don't think it reduces cache evictions the way it's described, and in some cases it increases them.
Autosaves, revisions, and draft saves are not avoided. wp_insert_post() calls clean_post_cache() and fires save_post unconditionally, and _wp_put_post_revision() inserts revisions via wp_insert_post( $post, true ). Both the old and new hooks fire for revisions and autosaves, so there's no difference for those.
wp_delete_user() (when reassigning the deleted user's posts) and add_ping() both call clean_post_cache(), which would be new cache flush since neither triggers save_post currently.
clean_post_cache fires on comment activity, which doesn't happen on save_post/delete_post. wp_update_comment_count_now() writes the post's comment_count and calls clean_post_cache() every time a comment is approved, unapproved, spammed, trashed, or deleted. On a site with active comments this would be a frequent new eviction, and comment counts don't affect calendar output.
I think the comment thing is the biggest potential problem with the change as is. I wonder if using transition_post_status and only deleting when old/new status was publish would be tighter?
#15
@
6 weeks ago
Thanks @mindctrl, good catches - you're right on both.
The autosave/revision bit: yeah, my claim was wrong. wp_insert_post() fires save_post and calls clean_post_cache() for the same events, revisions included, so switching the hook never actually skipped those flushes.
The comment activity is the real problem though. wp_update_comment_count_now() calls clean_post_cache() on every approve/unapprove/spam/trash/delete. So the calendar cache was getting flushed on comment changes that don't touch it at all. That's worse than what it replaced.
I've reworked it the way you suggested. Invalidation now hooks transition_post_status and only clears the cache when a post enters or leaves publish - so publish, unpublish, trash, and date edits on a published post. Plus a before_delete_post hook gated on publish, for forced deletes that skip the trash. The comment-count, autosave and draft-save flushes are all gone now.
Added tests so it stays that way: publishing and deleting a published post invalidate the cache, comment activity (approve/hold/spam) and draft saves don't. Full get_calendar suite passes, 13 tests. Updated in PR #11547.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Trac ticket: https://core.trac.wordpress.org/ticket/61343