WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 20 months ago

Last modified 19 months ago

#37464 closed enhancement (fixed)

Last changed helper function

Reported by: spacedmonkey Owned by: jorbin
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch needs-unit-tests
Focuses: performance Cc:

Description

The last changed value is used to invalidate many caches in core. Getting the code to last changed value is repeated through the core.

Attachments (3)

37464.diff (6.0 KB) - added by spacedmonkey 2 years ago.
37464.1.diff (6.1 KB) - added by spacedmonkey 2 years ago.
37464.2.diff (6.3 KB) - added by desrosj 21 months ago.

Download all attachments as: .zip

Change History (20)

@spacedmonkey
2 years ago

#1 follow-up: @DrewAPicture
2 years ago

  • Keywords has-patch 4.7-early added
  • Milestone changed from Awaiting Review to Future Release

@spacedmonkey: Thanks for the patch. Looks like 37464.diff is missing inline documentation. Also, we should follow current naming conventions and add wp_get_ to the front of the function name.

@spacedmonkey
2 years ago

This ticket was mentioned in Slack in #core by helen. View the logs.


22 months ago

#3 in reply to: ↑ 1 @SergeyBiryukov
22 months ago

  • Milestone changed from Future Release to 4.7

Replying to DrewAPicture:

Also, we should follow current naming conventions and add wp_get_ to the front of the function name.

Would wp_cache_get_last_changed() be a more descriptive name? 37464.1.diff seems good otherwise.

#4 @boonebgorges
22 months ago

The fact that the incrementor is tied to the "last change" seems like an implementation detail that shouldn't matter to people building cache keys. The important fact is that it's an incrementor. What do people think about wp_get_cache_incrementor()?

#5 @jorbin
21 months ago

I think wp_cache_get_last_changed() sounds like the best name. It is an implementation detail, but I think this function is clearly tied to implementation and is not overly generalized.

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


21 months ago

#7 @chriscct7
21 months ago

  • Keywords needs-unit-tests added

#8 @chriscct7
21 months ago

  • Keywords needs-refresh added; 4.7-early removed

Docbloc for new function is also incomplete

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


21 months ago

@desrosj
21 months ago

#10 @desrosj
21 months ago

  • Keywords needs-refresh removed

Updated patch changes the function name to wp_cache_get_last_changed() and updates the Docblock.

Would this function be something that needs to be pluggable for when other caching mechanisms are in use? If so, then maybe this should go in cache.php instead of function.php.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


21 months ago

This ticket was mentioned in Slack in #core by desrosj. View the logs.


20 months ago

#13 @jorbin
20 months ago

  • Owner set to jorbin
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core by desrosj. View the logs.


20 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


20 months ago

#16 @jorbin
20 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38849:

Cache API: introduce wp_cache_get_last_changed to improve DRY

One thing fairly common to the cache groups is a block of code to look to see when the cache was last changed, and if there isn't one, to set it for the current microtime(). It appears in 8 different places in core. This adds a new helper wp_cache_get_last_changed to DRY things up a bit.

Since wp-includes/cache.php isn't guaranteed to be loaded, this new function is in wp-includes/functions.php

Props spacedmonkey, desrosj.
Fixes #37464.

#17 @johnjamesjacoby
19 months ago

Is it too late in 4.7 to consider a wp_cache_set_last_changed()?

wp_cache_set( 'last_changed', microtime(), $foo ); is also used 11 times in core.

Note: See TracTickets for help on using tickets.