-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Lazy load post meta #4216
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
base: trunk
Are you sure you want to change the base?
Lazy load post meta #4216
Changes from 5 commits
e77768b
a9e45a1
b5f297e
efcdbc7
85ad4ae
9f19cac
77fa22e
753379d
093b224
ef8e797
b2e72e1
657aebb
ee9e65e
14aec68
d6d9934
8f33b9a
e61d194
55b32c3
4330c69
4f90281
046cdf5
cbe16f1
dc7344a
a077f46
f4ec367
59a3aee
a6b8d34
4e3a8ec
79f93c1
434cb55
95df19a
ebf588e
7893aee
01f785d
34855d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -797,9 +797,8 @@ public function fill_query_vars( $query_vars ) { | |||||||||
| * disable cache priming for term meta, so that each | ||||||||||
| * get_term_meta() call will hit the database. | ||||||||||
| * Defaults to the value of `$update_post_term_cache`. | ||||||||||
| * @type bool $lazy_load_post_meta Whether to lazy-load post meta. Setting to false will | ||||||||||
| * disable cache priming for post meta, so that each | ||||||||||
| * get_post_meta() call will hit the database. | ||||||||||
| * @type bool $lazy_load_post_meta Whether to lazy-load post meta. If set to false, post meta priming will | ||||||||||
| * fallback to the default behavior of priming post meta. | ||||||||||
|
Comment on lines
+800
to
+801
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the following changes better reflect the current state of the logic and resolves conversations here and here.
Suggested change
|
||||||||||
| * @type int $w The week number of the year. Default empty. Accepts numbers 0-53. | ||||||||||
| * @type int $year The four-digit year. Default empty. Accepts any four-digit year. | ||||||||||
| * } | ||||||||||
|
|
@@ -1295,7 +1294,7 @@ public function parse_tax_query( &$q ) { | |||||||||
| if ( ! empty( $q['category__not_in'] ) ) { | ||||||||||
| $q['category__not_in'] = array_map( 'absint', array_unique( (array) $q['category__not_in'] ) ); | ||||||||||
| sort( $q['category__not_in'] ); | ||||||||||
| $tax_query[] = array( | ||||||||||
| $tax_query[] = array( | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is one that is not applying because it was already fixed |
||||||||||
| 'taxonomy' => 'category', | ||||||||||
| 'terms' => $q['category__not_in'], | ||||||||||
| 'operator' => 'NOT IN', | ||||||||||
|
|
@@ -1364,7 +1363,7 @@ public function parse_tax_query( &$q ) { | |||||||||
| if ( ! empty( $q['tag__not_in'] ) ) { | ||||||||||
| $q['tag__not_in'] = array_map( 'absint', array_unique( (array) $q['tag__not_in'] ) ); | ||||||||||
| sort( $q['tag__not_in'] ); | ||||||||||
| $tax_query[] = array( | ||||||||||
| $tax_query[] = array( | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the second that is not applying |
||||||||||
| 'taxonomy' => 'post_tag', | ||||||||||
| 'terms' => $q['tag__not_in'], | ||||||||||
| 'operator' => 'NOT IN', | ||||||||||
|
|
@@ -2000,9 +1999,12 @@ public function get_posts() { | |||||||||
| $q['update_post_meta_cache'] = true; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Default lazy loading post meta to false, as this `update_post_meta_cache` defaults to true. | ||||||||||
| // To correctly use the lazy_load_post_meta parameter, you must set update_post_meta_cache to false first. | ||||||||||
| if ( ! isset( $q['lazy_load_post_meta'] ) ) { | ||||||||||
| $q['lazy_load_post_meta'] = false; | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per the DocBlock, shouldn't this be the following?
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than just removing the docs about the default in 4e3a8ec, can it be updated to be accurate?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joemcgill I am not sure what is being asked here. |
||||||||||
| } elseif ( $q['lazy_load_post_meta'] ) { | ||||||||||
| // Lazy loading post meta only works if post meta caches are not primed. | ||||||||||
| $q['update_post_meta_cache'] = false; | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For term meta, lazy loading requires term cache priming. Can you add an inline comment about why this is different? Also, the Docblock states...
...but that doesn't seem to be implemented.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @spacedmonkey it still doesn't seem that this comment was addressed:
Am I missing something?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joemcgill I have added some inline comments. |
||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spacedmonkey Re #4216 (comment), I think the code should simply be this to see if a database query is required. If not, it bypasses processing the queue, if so it adds the object ID and continues lazy loading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterwilsoncc This happens in here
wordpress-develop/src/wp-includes/meta.php
Lines 1168 to 1189 in bcb59bf
So if it first checks if the id is in cache, then if not, then it will do a database query.
Add this check, just repeats to the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not quite the same.
get_metadata( type, **cached** object ID ...)will process the queue, even though a database query is not required. This is because"get_{$meta_type}_metadata"fires prior to checking the objects cache.As you've been trying to avoid unnecessary database queries while getting post meta data, this acheives that by only processing the queue if a database query will be made.