3
\$\begingroup\$

I'm developing a fully AJAX community WP theme that comes with a lot of custom queries.

Everything is fine, there is no bug but recently but I had a few clients that have a huge database and my theme started to kill the CPU. I wonder how can I make it run faster.

What this code is doing?

This code is trying to get posts that contains only have "comment" type comments (comment_type="comment") AND approved ones.

database information: Mysql - mariaDB

table name I'm trying to get result - wp_comments

how many data that table have - 380.000 (380K) comments

query's raw output time on phpmyadmin (w/o php) - 0.23 seconds

time to get this data on front end - 0.85-90 seconds

JS Code that calls function

    function lfload(page) {
  $("#sol-load").css("opacity","0.75");
  document.cookie = "lf= popular;  expires = Fri, 31 Dec 9999 23:59:59 GMT; path=/";
  document.cookie = "popular_page = " + page + ";  expires = Fri, 31 Dec 9999 23:59:59 GMT; path=/";
  var xhr = new XMLHttpRequest();
  xhr.open("POST", bilgi.tema_url + '/admin-ajax.php', true);
  xhr.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
  xhr.onreadystatechange = function() {
    if (this.readyState === XMLHttpRequest.DONE && this.status === 200) {
      document.getElementById("sol-load").innerHTML = this.response;
      $("#sol-load").css("opacity","1");
  }
  }
  xhr.send("action=popular_ajax");
}

PHP Code to get data

function popular_ajax()
  {
          global $wpdb;
           // getting latest page from cookie
          if (isset($_COOKIE["popular_page"]) AND 
          $_COOKIE["popular_page"] != 0) {
              $latest_sent_page_no = intval($_COOKIE["popular_page"]);
          } elseif (!isset($_COOKIE["popular_page"]) OR
           $_COOKIE["popular_page"] == 0) {
              $latest_sent_page_no = 1;
          }
          $page = $latest_sent_page_no;
          $cur_page = $page;
          $page -= 1;
          $per_page = 20;
          $start = $page * $per_page;
          $count_limiti = $per_page*50; 
          // tried to limit count 
          //query to make it more faster but did'nt work
          // count how many page exists to use it for pagination
          $count = $wpdb->get_var("SELECT COUNT(DISTINCT comment_post_ID)
           FROM ".$wpdb->prefix."comments
           USE INDEX (left_frame_index) 
           WHERE comment_approved = 1 
           AND comment_type = 'comment' 
           LIMIT $count_limiti");
          $sorgu = $wpdb->get_results("SELECT DISTINCT comment_post_ID 
            FROM ".$wpdb->prefix."comments 
          USE INDEX (left_frame_index) 
          WHERE comment_approved = 1
           AND comment_type = 'comment' 
           GROUP BY comment_post_ID 
           ORDER BY MAX(comment_date) 
           DESC LIMIT $start, $per_page");
          // GET DATA FROM SQL
          foreach($sorgu as $goflying2){
              // Çıktı bölgesi
              $msg .= "<li class='has-border-bottom'><a class='pr-0 pl-0 pt-1 
              pb-2 is-size-8'
               href='". get_permalink($goflying2->comment_post_ID) ."'
                title='".get_the_title($goflying2->comment_post_ID)."'>
                ".get_the_title($goflying2->comment_post_ID)."<span class='badge'>
                " .clean_comment_count_wo_newbies($goflying2->comment_post_ID). 
                "</span></a></li>";
          }
          wp_reset_postdata();
          // SAY THERE IS NO DATA IF ITS EMPTY
          if ($count == 0) {
            echo '<div class="tag has-text-centered has-fullwidth">
            <a class="pr-0 pl-0 pt-2 pb-2 is-size-8 has-text-dark">
            '.__("gündemimiz boş...", 'hype-community').'</a></div>';
              exit;
          }
          // PAGINATOIN STARTING HERE
          $no_of_paginations = ceil($count / $per_page);
          $start_loop = 1;
          $end_loop = $no_of_paginations;
          //conditional pagination
          if ($cur_page > 1) {
              $pre = $cur_page - 1;
              $pag_container .= "
              <button onclick='gundemNav(this)' value='$pre' class='button is-small'>
                <i class='fa fa-angle-left' aria-hidden='true'></i>
              </button>
              ";
          }
  
          $pag_container .= "
          <div class='dropdown is-hoverable has-fullwidth'>
        <button class='button is-small ml-3 mr-3 dropdown-trigger 
        has-fullwidth' aria-haspopup='true' aria-controls='dropdown-menu5'>
           <span>$cur_page</span>
           <span class='icon'>
    <i class='fa fa-caret-down'></i>
         </span>
            </button>
              ".'<div class="dropdown-menu" id="dropdown-menu5" role="menu">
      <div class="dropdown-content">';
      // tried to limit for loop
          if ($end_loop > 50) {
              $end_loop = 50;
          }
          for ($i = $start_loop; $i <= $end_loop; $i++) {
            // loop to print all page numbers
              if ($cur_page == $i) {
                  $pag_container .= "<a value='$i' class='dropdown-item 
                  is-active'>$i</a>";
              } else {
                  $pag_container .= "<a onclick='gundemNav(this)' value='$i'
                   class='dropdown-item'>$i</a>";
              }
          }
          $pag_container = $pag_container . "
  </div></div>
          </div>";
          if ($cur_page < $no_of_paginations) {
            //conditional pagination output
              $nex = $cur_page + 1;
              $pag_container .= "
              <button onclick='gundemNav(this)' value='$nex' class='button is-small'>
                <i class='fa fa-angle-right' aria-hidden='true'></i>
              </button>
              ";
          }
          if ($no_of_paginations == 1) {
            //conditional pagination output
              print     '<aside class="menu"><ul class="menu-list">' . $msg . 
              '</ul></aside>';
          } elseif ($cur_page == 1 and $no_of_paginations >= 2) {
            //conditional pagination output
              print     '<aside class="menu"><ul class="menu-list">' . $msg . 
              '</ul></aside>';
              echo  '
              <button onclick="gundemNav(this)" value="'. $nex .'" 
              class="button is-small is-bg-blue has-text-white is-fullwidth mt-3">
                <span class="icon">
                  <i class="fa fa-book" aria-hidden="true"></i>
                </span>
                <strong>'.__("fazlasını yükle", 'hype-community').'</strong>
              </button>
              ';
          } elseif ($cur_page != 1 and $no_of_paginations > 1) {
              //conditional pagination output
              echo
           '<div class="has-text-centered is-flex mb-3">'.$pag_container .
            '</div>'. // pagination
          '<aside class="menu"><ul class="menu-list">' . $msg . 
          '</ul></aside>'; // content that called from wpdb get results
          }
          // kill ajax
          exit;
  }

INDEX I used in this query

left_frame_index -> commenst_post_ID, comment_date, comment_approved, comment_type

How can I make it run faster, be more stable? Is there something wrong with my SQL code or PHP code? This code is really killing the CPU...

Without this index, query is around 0.5 seconds (phpymyadmin exec. time. 1.5 seconds when calling with AJAX).

sorry for my bad English! ^_^

\$\endgroup\$
7
  • 1
    \$\begingroup\$ Please, format long lines to fit in one screen. \$\endgroup\$
    – Sugar
    Oct 9, 2020 at 15:52
  • \$\begingroup\$ hello! yes, sorry. I formatted my code. \$\endgroup\$
    – techn9ne
    Oct 9, 2020 at 17:04
  • \$\begingroup\$ Why do you tell the queries to use a specific index? Databases are usualy smart enough to decide on their own what index to use if any to get the best performance for any given query. \$\endgroup\$
    – slepic
    Oct 10, 2020 at 3:07
  • \$\begingroup\$ tbh I wanted to be sure but I was trying to fix this query last 10 hours. Now my query working exec time 0.056 seconds. But IDK when I'm trying spamming this ajax button with my mouse; my cpu goes crayz. I set a transient to count query (deleting it with new comments) but cpu still get hurts when I'm spamming it. But I switched to twentynineteen theme and spammed my f5 @home page and cpu went to %60 70 again. This makes me think everything is OK with my code but there is a problem with my hosting. \$\endgroup\$
    – techn9ne
    Oct 10, 2020 at 3:36
  • \$\begingroup\$ You're using both DISTINCT comment_post_ID and GROUP BY comment_post_ID in your query. Either one will be enough. I doubt this will affect CPU usage, but it is worth testing which variant is performs best. See also: stackoverflow.com/questions/581521/… \$\endgroup\$ Oct 10, 2020 at 13:47

2 Answers 2

2
\$\begingroup\$
SELECT  COUNT(DISTINCT comment_post_ID)
    FROM  ".$wpdb->prefix."comments USE INDEX (left_frame_index)
    WHERE  comment_approved = 1
      AND  comment_type = 'comment'
    LIMIT  $count_limit

Needs a better index; see the below.

Do NOT compare a VARCHAR (such as comment_approved) with a numeric literal (such as 1), the index cannot be used. Put quotes around "1" or '1'.

Get rid of the "index hint"; it may be hurting more than helping.

Why no ORDER BY? A limit without an order-by give you "random" rows.

Oops, the real problem is that the query generates only 1 row, so there is no use in having a LIMIT. Keep the DISTINCT; toss the LIMIT.

SELECT  DISTINCT comment_post_ID
    FROM  ".$wpdb->prefix."comments USE INDEX (left_frame_index)
    WHERE  comment_approved = 1
      AND  comment_type = 'comment'
    GROUP BY  comment_post_ID
    ORDER BY  MAX(comment_date) DESC
    LIMIT  $start, $per_page

See above, plus

Don't mix GROUP BY and DISTINCT. Use only the GROUP BY.

This query needs a different index (use the specified order):

INDEX(comment_type, comment_approved, comment_post_ID, comment_date)

I suspect this query is not giving you what you expect? Please describe in English what the goal is.

Pagination via OFFSET has problems. See http://mysql.rjweb.org/doc.php/pagination

\$\endgroup\$
1
\$\begingroup\$

Overall comments

Overall I’d say that the practice of sending HTML in an AJAX response and using that to directly set the content of a DOM element is an antiquated practice, and could be an XSS avenue. While the practice of sending data from the API and having the front end code construct the HTML dynamically may have been a new construct eight years ago it is much more common in today’s web.

Improving speed of code to lookup posts

To answer your question "How can I make it more faster, more stable." it looks like there is a query to get IDs for each post, and then in the loop to create list items, there are calls to get_permalink(), get_the_title() and clean_comment_count_wo_newbies(). Do those function calls run queries against the database? If so, that would likely be the bottleneck. Ideally the code would run a single query to get all the information needed - e.g. IDs, titles, comment counts, etc. Remember that DB queries are expensive so it is best to minimize the number of queries needed.

JavaScript

DOM Access

The code looks up the element with id sol-load three times within a few lines. It would be wise to cache those lookups, since they can be expensive.

bridge toll

”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the other, and every time I want to access the DOM from the JavaScript engine, I have to pay that toll”
    - John Hrvatin, Microsoft, MIX09, in this talk Building High Performance Web Applications and Sites at 29:38, also cited in the O'Reilly Javascript book by Nicholas C Zakas Pg 36, as well as mentioned in this post

const solLoad = $("#sol-load");

AJAX

The front end code uses jQuery to access DOM elements yet the AJAX code uses vanilla XHR mechanisms. The jQuery AJAX methods like $.get() and $.post() could be used to simplify the code. Actually .load() can be used to simplify the code dramatically -

I haven't tested this but this should be what would be needed:

solLoad.load(bilgi.tema_url + '/admin-ajax.php', {action: 'popular_ajax'}, 
    solLoad.css.bind(solLoad, "opacity", 1));

Inline event handlers

There are event handlers registered within the HTML code - e.g.

<button onclick='gundemNav(this)'

It is better to register event handlers within the JavaScript (e.g. using button.addEventListener (can be done when element is created or after is is selected via DOM) for multiple reasons:

  1. The logic can be separated from the markup - if multiple teammates worked on the project then one could work on the JavaScript while the other could work on the HTML independently.
  2. Such handlers can pollute the global namespace which can lead to strange behavior.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.