How to optimize this function?

Resolved Doug
(@nicoblog)

8 years, 2 months ago

Hi, i’m not a coder. But i made function to append total post count to the menu on multisite. I’m seeing rather high load on the VPS so i want to know if this function could be the culprit or not and if so, how can i optimize it?
This is the code:
http://pastebin.com/7QBie8Cj

Please remember i’m not a coder and what i’m doing is probably ridiculous but i don’t know any other way to achieve same result. If you do please let me know.
Thanks in advance.

Viewing 8 replies - 1 through 8 (of 8 total)

(@aut0poietic)

There’s a pretty nice example on stack exchange here: http://wordpress.stackexchange.com/questions/66963/how-to-display-network-post-count

That post is doing what you are ( with a few details changed ) and appears to address a memory leak issue with switch_to_blog(). However, querying multiple blogs is an expensive operation by nature. Another part of the solution would be to store the information in a transient — essentially a variable that expires after a set amount of time or when flagged as invalid.

Your function would need to 1) check to see if there was a transient 2) if their is,output the data from there 3) if not, rebuild the transient and output the new data.

In addition, you’d have to add an action in save_post for all your blogs that purges the transient any time a post is published.

More information to get you started:

https://codex.wordpress.org/Plugin_API/Action_Reference/save_post
https://codex.wordpress.org/Transients_API

Thread Starter

Doug

(@nicoblog)

8 years, 2 months ago

I think that’s too much for me.. can you tell me if this would work?

register_activation_hook(__FILE__, 'my_activation');

function my_activation() {
	wp_schedule_event(time(), 'daily', 'my_daily_event');
}

add_action('my_daily_event', 'do_this_daily');

function do_this_daily() {
	$original_blog_id = get_current_blog_id();
switch_to_blog(2);
$count_posts = wp_count_posts();
$site1 = $count_posts->publish;
switch_to_blog(3);
$count_posts = wp_count_posts();
$site2 = $count_posts->publish;
switch_to_blog(15);
$count_posts = wp_count_posts();
$site3 = $count_posts->publish;
switch_to_blog(18);
$count_posts = wp_count_posts();
$site4 = $count_posts->publish;
switch_to_blog(20);
$count_posts = wp_count_posts();
$site5 = $count_posts->publish;
switch_to_blog(22);
$count_posts = wp_count_posts();
$site6 = $count_posts->publish;
switch_to_blog(21);
$count_posts = wp_count_posts();
$site7 = $count_posts->publish;
switch_to_blog(6);
$count_posts = wp_count_posts();
$site8 = $count_posts->publish;
switch_to_blog(16);
$count_posts = wp_count_posts();
$site9 = $count_posts->publish;
switch_to_blog(17);
$count_posts = wp_count_posts();
$site10 = $count_posts->publish;
switch_to_blog(19);
$count_posts = wp_count_posts();
$site11 = $count_posts->publish;
switch_to_blog( $original_blog_id );
}

function prefix_nav_description( $item_output, $item, $depth, $args ) {

       $item_output = str_replace( "Volvo", "Volvo ($site1)", $item_output );
	   $item_output = str_replace( "Mazda", "Mazda ($site2)", $item_output );
	   $item_output = str_replace( "BMW", "BMW ($site3)", $item_output );
	   $item_output = str_replace( "Mercedes", "Mercedes ($site4)", $item_output );
	   $item_output = str_replace( "Porsche", "Porsche ($site5)", $item_output );
	   $item_output = str_replace( "Mitsubishi", "Mitsubishi ($site6)", $item_output );
	   $item_output = str_replace( "Hyundai", "Hyundai ($site7)", $item_output );
	   $item_output = str_replace( "Fiat", "Fiat ($site8)", $item_output );
	   $item_output = str_replace( "Ferrari", "Ferrari ($site9)", $item_output );
	   $item_output = str_replace( "Citroen", "Citroen ($site10)", $item_output );
	   $item_output = str_replace( "Renault", "Renault ($site11)", $item_output );

    return $item_output;
}
add_filter( 'walker_nav_menu_start_el', 'prefix_nav_description', 10, 4 );

Moderator

bcworkz

(@bcworkz)

8 years, 2 months ago

There’s a variable scope issue you have not addressed. The $site1 etc. variables set in the daily task are not available in the nav menu filter. Declaring them as globals ( a typical hacky shortcut ) will not even work because the variables occur in completely different processes.

You need to store the variables somewhere. aut0poietic’s suggestion of storing as a transient is a good one. If you address this I think your code will more or less work. There’s room for improvement, but it should at least work.

One improvement would be to store the counts in a single array instead of several variables, then they can be easily kept under a single transient value. For example, instead of $site1 use $doug_site[1], etc. Then all counts are in the $doug_site array, which can be stored and retrieved like a single variable, but all values are available within it.

I prepended “doug” to the variable name because “site” is a generic word which could clash with other code variables. Prepending a non-generic string makes your variable name much less prone to name conflicts. Of course it doesn’t need to be “doug”, any unique string will work.

Thread Starter

Doug

(@nicoblog)

8 years, 2 months ago

@bcworkz, thanks for the input. I followed your post carefully and came up with new code, is this doing what i wanted? I mean only updating only every 24hs? What do you think of it?

function prefix_nav_description( $item_output, $item, $depth, $args ) {
$posts_counters = array(
    "Volvo" => 0,
    "Mazda" => 0,
	"BMW" => 0,
	"Mercedes" => 0,
	"Porsche" => 0,
	"Mitsubishi" => 0,
	"Hyundai" => 0,
	"Fiat" => 0,
	"Ferrari" => 0,
	"Citroen" => 0,
	"Renault" => 0,
);

if ( false === ( $posts_counters = get_transient( 'posts_counters' ) ) ) {

$original_blog_id = get_current_blog_id();
switch_to_blog(2);
$count_posts = wp_count_posts();
$posts_counters["Volvo"] = $count_posts->publish;
switch_to_blog(3);
$count_posts = wp_count_posts();
$posts_counters["Mazda"] = $count_posts->publish;
switch_to_blog(15);
$count_posts = wp_count_posts();
$posts_counters["BMW"] = $count_posts->publish;
switch_to_blog(18);
$count_posts = wp_count_posts();
$posts_counters["Mercedes"] = $count_posts->publish;
switch_to_blog(20);
$count_posts = wp_count_posts();
$posts_counters["Porsche"] = $count_posts->publish;
switch_to_blog(22);
$count_posts = wp_count_posts();
$posts_counters["Mitsubishi"] = $count_posts->publish;
switch_to_blog(21);
$count_posts = wp_count_posts();
$posts_counters["Hyundai"] = $count_posts->publish;
switch_to_blog(6);
$count_posts = wp_count_posts();
$posts_counters["Fiat"] = $count_posts->publish;
switch_to_blog(16);
$count_posts = wp_count_posts();
$posts_counters["Ferrari"] = $count_posts->publish;
switch_to_blog(17);
$count_posts = wp_count_posts();
$posts_counters["Citroen"] = $count_posts->publish;
switch_to_blog(19);
$count_posts = wp_count_posts();
$posts_counters["Renault"] = $count_posts->publish;
switch_to_blog( $original_blog_id );	 

     set_transient( 'posts_counters', $posts_counters, 24 * HOUR_IN_SECONDS );
}

       $item_output = str_replace( "Volvo", "Volvo (".$posts_counters["Volvo"].")", $item_output );
	   $item_output = str_replace( "Mazda", "Mazda (".$posts_counters["Mazda"].")", $item_output );
	   $item_output = str_replace( "BMW", "BMW (".$posts_counters["BMW"].")", $item_output );
	   $item_output = str_replace( "Mercedes", "Mercedes (".$posts_counters["Mercedes"].")", $item_output );
	   $item_output = str_replace( "Porsche", "Porsche (".$posts_counters["Porsche"].")", $item_output );
	   $item_output = str_replace( "Mitsubishi", "Mitsubishi (".$posts_counters["Mitsubishi"].")", $item_output );
	   $item_output = str_replace( "Hyundai", "Hyundai (".$posts_counters["Hyundai"].")", $item_output );
	   $item_output = str_replace( "Fiat", "Fiat (".$posts_counters["Fiat"].")", $item_output );
	   $item_output = str_replace( "Ferrari", "Ferrari (".$posts_counters["Ferrari"].")", $item_output );
	   $item_output = str_replace( "Citroen", "Citroen (".$posts_counters["Citroen"].")", $item_output );
	   $item_output = str_replace( "Renault", "Renault (".$posts_counters["Renault"].")", $item_output );

    return $item_output;
}
add_filter( 'walker_nav_menu_start_el', 'prefix_nav_description', 10, 4 );

Thanks.

Moderator

bcworkz

(@bcworkz)

8 years, 2 months ago

That looks good, nice work!

While there could be some minor syntax errors remaining, the basic logic appears sound. One thing to check for is that the str_replace() functions are only replacing what they are supposed to. Check that counts are not added to anything but what is visible on the page.

Another possible improvement would be which nav menu filter hook you are using. “Possible” because the following suggestion may not be feasible. The ‘start_el’ hook fires once for every item in the menu, so most of the str_replace() calls are superfluous for any given pass. Consider using a hook like ‘wp_nav_menu’ which only fires once for all menu items. This may not work if the strings to modify are not unique within the entire menu structure. The counts might end up where they don’t belong. But if this does work it’ll cut down on the overall processing required. It’s not necessary to do this, but it might be worth investigating.

Thread Starter

Doug

(@nicoblog)

8 years, 2 months ago

str_replace() was also replacing the “title” attribute of the menu items.
I solved this by typing the anchor text in upper case, changed the str_replace to match the strings in upper case only. Since the theme already has a css rule “text-transform: uppercase;” for this items nobody notice the difference.

Now i’m sure the replace only happens once for each item.

How would i go about using wp_nav_menu? Is this enough or i have to change something else too?

//change this line:
add_filter( 'walker_nav_menu_start_el', 'prefix_nav_description', 10, 4 );
//like this?
add_filter( 'wp_nav_menu_args', 'prefix_nav_description', 10, 4 );

Thanks!

Moderator

bcworkz

(@bcworkz)

8 years, 2 months ago

Sort of, that’s almost it. The filter is ‘wp_nav_menu’, not ‘wp_nav_menu_args’.

Also change the 4 to a 2 and remove $item, $depth, from the function declaration:
function prefix_nav_description( $item_output, $args ) {

This is all assuming that your theme uses wp_nav_menu(), which is not always the case.

Thread Starter

Doug

(@nicoblog)

8 years, 2 months ago

I’ve made all changes suggested on your last post, the counters show up so i assume all went OK, it’s a twenty fourteen child so it should be coded by the book.
Thanks for the help!

PS: On the post linked by aut0poietic they are doing wp_cache_flush() before every switch_to_blog. I don’t know what that does but if it’s clearing the W3 total cache…then it would have a negative effect on busy server, the load would be huge it might even crash. So i’m not doing it.

Viewing 8 replies - 1 through 8 (of 8 total)

The topic ‘How to optimize this function?’ is closed to new replies.

How to optimize this function?

Tags

Topics

Topics with no replies

Non-support topics

Resolved topics

Unresolved topics

All topics