Optimising Code (too many DB queries)

Robust

Robust

Well-known member
Before: Timing: 0.0837 seconds - Memory: 8.908 MB - DB Queries: 10
After: Timing: 0.0904 seconds - Memory: 8.934 MB - DB Queries: 20

I get timing isn't created significantly, but with more add-ons it was doubled (from 0.1 to 0.2) and probably critical for bigger forums.

See https://www.apantic.com/community/threads/answered.1/?_debug=1

These are the classes that make DB queries:

PHP: 
<?php

class BestAnswer_Model_Vote extends XenForo_Model
{
    /**
     * Gets a best answer content record for a user that has voted on a piece of content.
     *
     * @param integer $postId
     * @param integer $userId
     *
     * @return array|false
     */
    public function getCurrentBestAnswerStatusOnPost($postId, $userId)
    {
        return $this->_getDb()->fetchRow('
            SELECT *
            FROM ba_votes
            WHERE post_id = ?
                AND vote_user_id = ?
        ', array($postId, $userId));
    }

    /**
     * Vote a post as best answer
     *
     * @param $postId int
     * @param $postUserId int
     * @param int|null $voteUserId
     * @param int|null $voteDate
     *
     * @return bool|false
     */
    public function votePost($postId, $postUserId, $voteUserId = null, $voteDate = null)
    {
        $visitor = XenForo_Visitor::getInstance();

        if($voteUserId === null)
        {
            $voteUserId = $visitor['user_id'];
        }
        if(!$voteUserId)
        {
            return false;
        }

        if($voteUserId != $visitor['user_id'])
        {
            /* @var $user XenForo_Model_User */
            $user = $this->getModelFromCache('XenForo_Model_User')->getUserById($voteUserId);

            if(!$user)
            {
                return false;
            }
            $voteUsername = $user['username'];
        }
        else
        {
            $voteUsername = $visitor['username'];
        }

        if($voteDate === null)
        {
            $voteDate = XenForo_Application::$time;
        }

        $post = $this->_getPostModel()->getPostById($postId);
        $threadId = $post['thread_id'];

        $db = $this->_getDb();
        XenForo_Db::beginTransaction($db);

        $query = $db->query('
            INSERT IGNORE INTO ba_votes
              (post_id, thread_id, vote_user_id, post_user_id, vote_date)
            VALUES
              (?, ?, ?, ?, ?)
        ', array($postId, $threadId, $voteUserId, $postUserId, $voteDate));

        if(!$query->rowCount())
        {
            XenForo_Db::commit($db);
            return false;
        }

        if($this->voteCountCheck($postId))
        {
            $this->recountVotesInThread($threadId);
        }

        $options = XenForo_Application::getOptions();
        if($options->baSetAnsweredPrefix)
        {
            $dwInput = array(
                'title' => new XenForo_Phrase($this->_getPrefixModel()->getPrefixTitlePhraseName($options->baSetAnsweredPrefix)),
                'prefix_id' => $options->baSetAnsweredPrefix
            );

            $forum = $this->_getForumModel()->getForumById($this->_getThreadModel()->getThreadById($threadId)['node_id']);

            if (!$this->_getPrefixModel()->verifyPrefixIsUsable($options->baSetAnsweredPrefix, $forum['node_id']))
            {
                $dwInput['prefix_id'] = 0;
            }

            $dw = XenForo_DataWriter::create('XenForo_DataWriter_Discussion_Thread');
            $dw->setExistingData($threadId);
            $dw->bulkSet($dwInput);
            $dw->setExtraData(XenForo_DataWriter_Discussion_Thread::DATA_FORUM, $forum);
            $dw->save();
        }

        if($postUserId)
        {
            /* @var $userModel XenForo_Model_User */
            $userModel = $this->getModelFromCache('XenForo_Model_User');
            $postUser = $userModel->getUserById($postUserId, array(
                'join' => XenForo_Model_User::FETCH_USER_OPTION | XenForo_Model_User::FETCH_USER_PROFILE
            ));

            if($postUser) {
                $db->query('
                    UPDATE `xf_user`
                    SET bestanswers = bestanswers + 1
                    WHERE user_id = ?
                ', $postUserId);

                if (!$userModel->isUserIgnored($postUser, $voteUserId)
                    && XenForo_Model_Alert::userReceivesAlert($postUser, 'post', 'vote'))
                {
                    $this->_getAlertModel()->alert(
                        $postUserId,
                        $voteUserId,
                        $voteUsername,
                        'post',
                        $postId,
                        'vote'
                    );
                }
            }
        }

        $this->_getNewsFeedModel()->publish(
            $voteUserId,
            $voteUsername,
            'post',
            $postId,
            'vote'
        );

        XenForo_Db::commit($db);
        return true;
    }

    public function unvotePost(array $vote)
    {
        $db = $this->_getDb();
        XenForo_Db::beginTransaction($db);

        $query = $db->query('
            DELETE FROM ba_votes
            WHERE vote_id = ?
        ', $vote['vote_id']);

        if(!$query->rowCount())
        {
            XenForo_Db::commit($db);
            return false;
        }

        $post = $this->_getPostModel()->getPostById($vote['post_id']);
        $threadId = $post['thread_id'];

        $this->recountVotesInThread($threadId);

        if($vote['post_user_id'])
        {
            $db->query('
                UPDATE `xf_user`
                SET bestanswers = bestanswers - 1
                WHERE user_id = ?
            ', $vote['post_user_id']);

            $this->_getAlertModel()->deleteAlerts(
                'post',
                $vote['post_id'],
                $vote['vote_user_id'],
                'vote'
            );
        }

        $this->_getNewsFeedModel()->delete(
            'post',
            $vote['post_id'],
            $vote['vote_user_id'],
            'vote'
        );

        XenForo_Db::commit($db);
        return true;
    }

    public function recountVotesInThread($threadId)
    {
        $db = $this->_getDb();

        $result = $db->fetchRow('
            SELECT COUNT(*) AS num_votes, ba_votes.post_id
            FROM ba_votes
              INNER JOIN xf_post
              ON ba_votes.post_id = xf_post.post_id
            WHERE ba_votes.thread_id = ?
            GROUP BY post_id
            ORDER BY num_votes desc
            LIMIT 1
        ', $threadId);

        $postId = $result['post_id'];

        if(empty($postId))
        {
            $postId = 0;
        }

        $db->query('
            UPDATE xf_thread
            SET bestanswer = ?
            WHERE thread_id = ?
        ', array($postId, $threadId));
    }

    public function voteCountCheck($postId)
    {
        $options = XenForo_Application::getOptions();
        if($options->baMinimumVotes)
        {
            $db = $this->_getDb();

            $result = $db->fetchRow('
            SELECT COUNT(*) AS num_votes, ba_votes.post_id
            FROM ba_votes
              INNER JOIN xf_post
              ON ba_votes.post_id = xf_post.post_id
            WHERE ba_votes.post_id = ?
            GROUP BY post_id
            ORDER BY num_votes desc
        ', $postId);

            $postId = $result['post_id'];

            if(empty($postId))
            {
                return new XenForo_Exception('Error getting vote count. Post ID is empty.');
            }

            return ($result['num_votes'] >= $options->baMinimumVotes) ? true : false;
        }
        return true;
    }

    public function canVotePost(array $post, array $thread, array $fourm, &$errorPhraseKey = '', array $nodePermissions = null, array $viewingUser = null)
    {
        $this->standardizeViewingUserReferenceForNode($thread['node_id'], $viewingUser, $nodePermissions);

        if (!$viewingUser['user_id'])
        {
            return false;
        }

        if ($post['message_state'] != 'visible')
        {
            return false;
        }

        if(!$this->votingEnabledInForum($thread['node_id']))
        {
            $errorPhraseKey = 'voting_not_enabled_in_forum';
            return false;
        }

        if ($post['user_id'] == $viewingUser['user_id'])
        {
            $errorPhraseKey = 'voting_own_content_not_allowed';
            return false;
        }

        if($post['post_id'] == $thread['first_post_id'])
        {
            $errorPhraseKey = 'cant_vote_first_post_in_thread';
            return false;
        }

        return XenForo_Permission::hasContentPermission($nodePermissions, 'vote');
    }

    public function votingEnabledInForum($forumId)
    {
        $options = XenForo_Application::getOptions();
        $enabledForums = $options->baEnabledForums;

        return (in_array($forumId, $enabledForums) ? true : false);
    }

    /**
     * @return XenForo_Model_Alert
     */
    protected function _getAlertModel()
    {
        return $this->getModelFromCache('XenForo_Model_Alert');
    }

    /**
     * @return XenForo_Model_NewsFeed
     */
    protected function _getNewsFeedModel()
    {
        return $this->getModelFromCache('XenForo_Model_NewsFeed');
    }

    /**
     * @return XenForo_Model_Post
     */
    protected function _getPostModel()
    {
        return $this->getModelFromCache('XenForo_Model_Post');
    }

    /**
     * @return XenForo_Model_ThreadPrefix
     */
    protected function _getPrefixModel()
    {
        return $this->getModelFromCache('XenForo_Model_ThreadPrefix');
    }

    /**
     * @return XenForo_Model_Thread
     */
    protected function _getThreadModel()
    {
        return $this->getModelFromCache('XenForo_Model_Thread');
    }

    /**
     * @return XenForo_Model_Forum
     */
    protected function _getForumModel()
    {
        return $this->getModelFromCache('XenForo_Model_Forum');
    }
}

PHP: 
<?php

class BestAnswer_Helper_Time
{
    public static function getTimeByPostId($postId)
    {
        if($postId)
        {
            $db = XenForo_Application::getDb();
            $voteDate = $db->fetchOne('
                SELECT vote_date
                FROM ba_votes
                WHERE post_id = ?
            ', $postId);

            return $voteDate;
        }
        return false;
    }
}

See https://www.apantic.com/community/threads/answered.1/?_debug=1
This is a copy without any other add-ons enabled.

Queries created by this add-on:
  1. SELECT vote_date
    FROM ba_votes
    WHERE post_id = ?
    Params: 2
    Run Time: 0.000078
    Select Type Table Type Possible Keys Key Key Len Ref Rows Extra
    SIMPLE ba_votes ALL 1 Using where
  2. SELECT *
    FROM ba_votes
    WHERE post_id = ?
    AND vote_user_id = ?
    Params: 1, 1
    Run Time: 0.000191
    Select Type Table Type Possible Keys Key Key Len Ref Rows Extra
    SIMPLE ba_votes ALL 1 Using where
  3. SELECT *
    FROM ba_votes
    WHERE post_id = ?
    AND vote_user_id = ?
    Params: 1, 1
    Run Time: 0.000104
    Select Type Table Type Possible Keys Key Key Len Ref Rows Extra
    SIMPLE ba_votes ALL 1 Using where
  4. SELECT *
    FROM ba_votes
    WHERE post_id = ?
    AND vote_user_id = ?
    Params: 2, 1
    Run Time: 0.000113
    Select Type Table Type Possible Keys Key Key Len Ref Rows Extra
    SIMPLE ba_votes ALL 1 Using where
  5. SELECT *
    FROM ba_votes
    WHERE post_id = ?
    AND vote_user_id = ?
    Params: 2, 1
    Run Time: 0.000104
    Select Type Table Type Possible Keys Key Key Len Ref Rows Extra
    SIMPLE ba_votes ALL 1 Using where
Not sure about the other 5 queries and where they came from.

Code: 
Included Files (97, XenForo Classes: 61)

index.php
library/XenForo/Autoloader.php
library/XenForo/Application.php
library/Zend/Registry.php
library/Lgpl/utf8.php
library/Zend/Config.php
library/config.php
library/XenForo/FrontController.php
library/XenForo/Dependencies/Public.php
library/XenForo/Dependencies/Abstract.php
library/Zend/Controller/Request/Http.php
library/Zend/Controller/Request/Abstract.php
library/Zend/Uri.php
library/Zend/Controller/Response/Http.php
library/Zend/Controller/Response/Abstract.php
library/XenForo/Model/DataRegistry.php
library/XenForo/Model.php
library/Zend/Cache.php
library/Zend/Cache/Backend/Memcached.php
library/Zend/Cache/Backend/ExtendedInterface.php
library/Zend/Cache/Backend/Interface.php
library/Zend/Cache/Backend.php
library/Zend/Cache/Core.php
library/XenForo/CodeEvent.php
library/XenForo/Options.php
library/XenForo/Link.php
library/XenForo/Template/Helper/Core.php
library/BestAnswer/Listener/Helper.php
library/XenForo/Router.php
library/XenForo/Route/Filter.php
library/XenForo/Route/Interface.php
library/XenForo/Route/ResponseSuffix.php
library/XenForo/Route/Prefix.php
library/XenForo/Route/Prefix/Threads.php
library/XenForo/RouteMatch.php
library/XenForo/ControllerPublic/Thread.php
library/XenForo/ControllerPublic/Abstract.php
library/XenForo/Controller.php
library/XenForo/Input.php
library/XenForo/Session.php
library/Zend/Db.php
library/Zend/Db/Adapter/Mysqli.php
library/Zend/Db/Adapter/Abstract.php
library/Zend/Db/Select.php
library/Zend/Db/Expr.php
library/Zend/Db/Profiler.php
library/Zend/Db/Statement/Mysqli.php
library/Zend/Db/Statement.php
library/Zend/Db/Statement/Interface.php
library/XenForo/Helper/Ip.php
library/XenForo/Visitor.php
library/XenForo/Model/User.php
library/Zend/Db/Profiler/Query.php
library/XenForo/Permission.php
library/XenForo/Phrase.php
library/XenForo/Locale.php
library/XenForo/ControllerHelper/ForumThreadPost.php
library/XenForo/ControllerHelper/Abstract.php
library/XenForo/Model/Thread.php
library/XenForo/Model/Forum.php
library/XenForo/Helper/String.php
library/XenForo/Model/Post.php
library/XenForo/Model/Attachment.php
library/XenForo/Model/Node.php
library/XenForo/Route/Prefix/Categories.php
library/XenForo/Route/Prefix/Forums.php
library/XenForo/ControllerResponse/View.php
library/XenForo/ControllerResponse/Abstract.php
library/XenForo/ViewRenderer/HtmlPublic.php
library/XenForo/ViewRenderer/Abstract.php
library/XenForo/Template/Public.php
library/XenForo/Template/Abstract.php
library/XenForo/ViewPublic/Thread/View.php
library/XenForo/ViewPublic/Base.php
library/XenForo/View.php
library/XenForo/BbCode/Parser.php
library/XenForo/BbCode/Formatter/Base.php
library/XenForo/ViewPublic/Helper/Message.php
library/XenForo/BbCode/TextWrapper.php
library/XenForo/ViewPublic/Helper/Editor.php
library/XenForo/Route/Prefix/Members.php
library/XenForo/Template/FileHandler.php
library/XenForo/Helper/File.php
internal_data/templates/S.1,L.1,thread_view.php
library/BestAnswer/Helper/PostUser.php
library/BestAnswer/Helper/Time.php
library/BestAnswer/Helper/PostMessage.php
library/Zend/Debug.php
library/XenForo/Route/Prefix/Posts.php
library/BestAnswer/Helper/CheckVote.php
library/BestAnswer/Model/Vote.php
library/XenForo/Model/Avatar.php
library/XenForo/Route/Prefix/SpamCleaner.php
internal_data/templates/S.1,L.1,editor.php
library/XenForo/ViewRenderer/Json.php
library/XenForo/Debug.php
internal_data/templates/S.1,L.1,PAGE_CONTAINER.php
 
Might come from your getCurrentBestAnswerStatusOnPost as the queries seems similar.

If you are calling it for each post that is in the displayed thread, it might tends to get big...
You should find a way to gather the postIds and then retrieve the data from your ba_votes class.

Clément
 
MtoR said:
Might come from your getCurrentBestAnswerStatusOnPost as the queries seems similar.

If you are calling it for each post that is in the displayed thread, it might tends to get big...
You should find a way to gather the postIds and then retrieve the data from your ba_votes class.

Clément
Click to expand...
Yup, that's one problem. Thanks. It's ran once per post it seems. If I get all of the posts, I'm still going to have to do a foreach with a lot of queries to find out if the user has voted for a post or not.
 
Doesn't default XF's Like system do the same anyway?

Code: 
    /**
     * Gets a liked content record for a user that has liked a piece of content.
     *
     * @param string $contentType
     * @param integer $contentId
     * @param integer $userId
     *
     * @return array|false
     */
    public function getContentLikeByLikeUser($contentType, $contentId, $userId)
    {
        return $this->_getDb()->fetchRow('
            SELECT *
            FROM xf_liked_content
            WHERE content_type = ?
                AND content_id = ?
                AND like_user_id = ?
        ', array($contentType, $contentId, $userId));
    }

$likeModel = $this->_getLikeModel();
$existingLike = $likeModel->getContentLikeByLikeUser('post', $postId, XenForo_Visitor::getUserId());

I think the like system only uses it to determine whether to submit a like or an unlike. I'm using it to decide whether to use the phrase 'vote' or the phrase 'unvote'. Alternatives?
 
I am not sure it's meant for the same purpose.

Perhaps by connecting where the query get the content and doing a left join.
 
MtoR said:
I am not sure it's meant for the same purpose.

Perhaps by connecting where the query get the content and doing a left join.
Click to expand...
Not sure what you mean. And yeah, different purposes. I need a better solution to decide whether to show vote or unvote.
 
I meant that if your goal is to know wether a user voted for a post or not then you should try perhaps to hook on the posts retrieval query or fetchOptions (Post model I guess).

Your goal would then be to had some code in order to perform a left join in the sql query to get informations from your other table, to get something like :
Code: 
LEFT JOIN ba_votes AS bv ON (bv.post_id=post.post_id

And you'd have to add also some fields retrieval from your ba_votes table in the query fields.

Then you can use the result as conditional to see wether it contained vote entry or not.

Btw, if you are trying to check if a user voted, don't you lack of a user_id conditional in your query ?

Clément
 
MtoR said:
I meant that if your goal is to know wether a user voted for a post or not then you should try perhaps to hook on the posts retrieval query or fetchOptions (Post model I guess).

Your goal would then be to had some code in order to perform a left join in the sql query to get informations from your other table, to get something like :
Code: 
LEFT JOIN ba_votes AS bv ON (bv.post_id=post.post_id

And you'd have to add also some fields retrieval from your ba_votes table in the query fields.

Then you can use the result as conditional to see wether it contained vote entry or not.

Btw, if you are trying to check if a user voted, don't you lack of a user_id conditional in your query ?

Clément
Click to expand...
Current query:
return $this->_getDb()->fetchRow('
SELECT *
FROM ba_votes
WHERE post_id = ?
AND vote_user_id = ?
', array($postId, $userId));

Hm, I just understood what you mean. That could work, if it's the only option I guess.
 
Yes for the current query it's fine with the user_id.

I guess the only would indeed be to hook as I suggest.

Clément
 
I got one idea. Start recording thread_id in the table. Make one query to fetch everything in the thread, add the user ID in there too. Group by post_id. Then just check which post_ids the query returned? Not sure if that's better or worse.

If that's not as good of an idea, how would I even modify an editing model? Extending the class won't modify the existing model. Not sure if there are any listeners for this either.
 
Well your way might be working to, I have some knowledge in sql but it goes beyond my knowledge to tell you wether the one or other way is the best.

Anyhow, you can extend model through code event listeners.

Clément
 
You must log in or register to reply here.

Similar threads

DRE
How Many DB Queries Does Your Home Page Portal Run?
2 3
Replies
45
Views
4K
rdn
R
Back
Top Bottom