WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24445 closed enhancement (fixed)

Improve is_multi_author() performance

Reported by: alexvorn2 Owned by: nacin
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: Template Keywords: has-patch commit
Focuses: Cc:

Description

I have 100k posts.

If I delete the function twentythirteen_body_class() - the home page loads in 1 second,
with this function the page loads in 4.4 seconds!

I use this for page load time:

add_action( 'wp_footer',             'footer_timer_stop', 1000               );

// Footer timer to show page load speed
function footer_timer_stop() {
	echo '<!-- stats: ' . get_num_queries() . ' queries. ' . timer_stop( 0 ) . ' seconds. -->';
}


This function is not so important and I think it should be removed.

Attachments (1)

24445.diff (1.1 KB) - added by markjaquith 4 years ago.

Download all attachments as: .zip

Change History (18)

#1 @alexvorn2
4 years ago

inside this function we have is_multi_author() that cause the page to load so slow...

#2 @SergeyBiryukov
4 years ago

  • Component changed from Performance to Bundled Theme

#3 follow-up: @nacin
4 years ago

Care to tell us how long the query in is_multi_author() is taking? How many rows in your posts table?

This function is not new. It was used similarly in Twenty Eleven and Twenty Twelve as well.

I was thinking about an optimization the other day to is_multi_author(), but it would only help for multi-author blogs. Basically:

$is_multi_author = ( count( array_unique( wp_list_pluck( 'post_author', $wp_query->posts ) ) ) > 1 );

If that returns false, we'd have to do the query either way. That said, I find it hard to believe that query is taking a particularly long time.

#4 in reply to: ↑ 3 @alexvorn2
4 years ago

Replying to nacin:

Care to tell us how long the query in is_multi_author() is taking? How many rows in your posts table?

I take 3.4 seconds to load.
100000 rows in the wp_posts table.

#5 @lancewillett
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.6

I think we could get rid of custom-font, won't speed things up a ton, but it's not used anywhere in the theme.

#6 @lancewillett
4 years ago

In 24390:

Twenty Thirteen: remove unused custom-font body class value. See #24445.

#7 @lancewillett
4 years ago

  • Keywords needs-patch removed
  • Priority changed from normal to low

#8 @alexvorn2
4 years ago

the problem is in the is_multi_author() function, so I think the patch is not entirely resolving the problem.

#9 follow-up: @markjaquith
4 years ago

We could use a transient for storing this value instead of raw cache get/set. That way people without persistent object caching would still benefit from the caching of that value.

@markjaquith
4 years ago

#10 @markjaquith
4 years ago

Untested patch that converts to use transients API.

#11 @lancewillett
4 years ago

  • Component changed from Bundled Theme to Template
  • Priority changed from low to normal
  • Summary changed from Twenty Thirteen: twentythirteen_body_class function make the load more slower! to Improve is_multi_author() performance

Moving out of Bundle Theme component as it's a core template improvement.

#12 @alexvorn2
4 years ago

I tested the patch, works nice.

#13 @alex-ye
4 years ago

Nice improvement, for me the is_multi_author() doesn't take a lot time, but maybe it's related with the posts count.

In addition to the above patch I will love to see a new filter 'pre_is_multi_author' to set a predefined value so the function doesn't need to run the database query.

#14 in reply to: ↑ 9 @nacin
4 years ago

Replying to markjaquith:

We could use a transient for storing this value instead of raw cache get/set. That way people without persistent object caching would still benefit from the caching of that value.

Transients came up when we originally did this, see #14405. Reading back through the ticket, what tipped the scales was an assumption that:

  • This query was fast
  • Transients require more queries

However, an unexpiring transient is autoloaded. And this is the perfect use case for one, because we know when it needs to be cleared.

#15 follow-up: @nacin
4 years ago

  • Keywords has-patch commit added

westi, objections?

#16 @nacin
4 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 24628:

Switch to a transient for is_multi_author(). props markjaquith, fixes #24445.

#17 in reply to: ↑ 15 @westi
4 years ago

Replying to nacin:

westi, objections?

Nope, seems sane and sensible to me.

Note: See TracTickets for help on using tickets.