Opened 12 years ago
Closed 12 years ago
#21434 closed enhancement (fixed)
Don't obliterate the object cache in switch_to_blog()
Reported by: | ryan | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | 3.4.1 |
Component: | Cache API | Keywords: | |
Focuses: | Cc: |
Description
switch_to_blog() calls wp_cache_init() which destroys the current $wp_object_cache object and creates a new one. This can be really expensive, especially with the non-persistent default cache. Instead of reinitializing, let's introduce something like wp_cache_switch_to_blog(). The memcached backend already builds the blog id into keys not in global groups, making ::switch_to_blog() very easy to implement. The default cache can do the same (I think it was keyed by blog id once upon a time).
Attachments (10)
Change History (23)
#2
@
12 years ago
- Owner set to ryan
- Resolution set to fixed
- Status changed from new to closed
In [21403]:
#4
follow-up:
↓ 7
@
12 years ago
The new, efficient code for switching between blogs is an excellent improvement. Thanks.
Can you please elaborate on the reasoning behind deprecating wp_cache_reset()
/ WP_Object_Cache::reset()
and why the deprecation message instructs people to use
wp_cache_switch_to_blog()
/ switch_to_blog()
? The reset()
and switch_to_blog()
methods serve two separate purposes. (Though some people may have used them in conjunction.)
May I suggest the deprecation message direct people to wp_cache_flush()
/ flush()
instead or have the replacement parameter mention both the flush and switch methods (eg "flush() or switch_to_blog()
").
The reason I bring this up is I use wp_cache_reset()
in unit tests for my plugin to clean up the environment between tests so data is pulled from the database, not the cache. The existing deprecation message sent me on a wild goose chase in which I found this ticket, spent time composing this message (during which I thought to seek a similar method in the class, which led me to flush()
). All in all, a good chunk of time, wasted. If the message had directed me to the similarly functioning flush method, I'd have been on my way in moments.
Oh, and can you please add the @since 3.5.0
tags to the new function / method?
#6
@
12 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
21434.6.diff adds @since for wp_cache_switch_to_blog()
and WP_Object_Cache::switch_to_blog()
.
#7
in reply to:
↑ 4
;
follow-up:
↓ 9
@
12 years ago
Replying to convissor:
The new, efficient code for switching between blogs is an excellent improvement. Thanks.
Can you please elaborate on the reasoning behind deprecating
wp_cache_reset()
/WP_Object_Cache::reset()
and why the deprecation message instructs people to use
wp_cache_switch_to_blog()
/switch_to_blog()
? Thereset()
andswitch_to_blog()
methods serve two separate purposes. (Though some people may have used them in conjunction.)
wp_cache_reset() was only ever implemented by the default, built-in cache. The other backends did not implement it. Now that we have switch_to_blog() it is even more useless since core never calls it.
#9
in reply to:
↑ 7
@
12 years ago
Replying to ryan:
wp_cache_reset() was only ever implemented by the default, built-in cache. The other backends did not implement it. Now that we have switch_to_blog() it is even more useless since core never calls it.
Deprecating and removing the reset methods is fine. Even if the reset method was never used anywhere inside WP's core, the method was there for the general public to use. And they have, I'm living proof.
The existing deprecation messages point people to the switch_to_blog()
method, but that method does not clear the cache.
The deprecation messages needs to be updated, please, to point people to a proper replacement: flush()
.
#11
@
12 years ago
flush() is not the proper replacement. It does nothing with some backends, especially for multisite.
We don't want to encourage flushing/resetting the cache, we want to encourage switch_to_blog(). For unit tests that use the default backend, consider overwriting the cache object. wp_cache_flush() is fine, though, when testing a specific backend that is known to support flushing for both single and multisite.
I'll add a note to the phpdoc. Patch coming up.
First draft, untested