Zend\Paginator\Paginator::setItemCountPerPage() Performance improvement

[GeeH]

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html

---

Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7469
User: @fedys
Created On: 2015-04-29T12:04:42Z
Updated At: 2015-11-06T21:32:29Z
Body
Calling of Zend\Paginator\Paginator::setItemCountPerPage() calls immediately $this->getAdapter()->count(). This could be postponed by setting null to $this->pageCount instead of $this->_calculatePageCount()

before improvement:

public function setItemCountPerPage($itemCountPerPage = -1)
{
    $this->itemCountPerPage = (int) $itemCountPerPage;
    if ($this->itemCountPerPage < 1) {
        $this->itemCountPerPage = $this->getTotalItemCount();
    }
    $this->pageCount        = $this->_calculatePageCount();
    $this->currentItems     = null;
    $this->currentItemCount = null;

    return $this;
}

after improvement:

public function setItemCountPerPage($itemCountPerPage = -1)
{
    $this->itemCountPerPage = (int) $itemCountPerPage;
    if ($this->itemCountPerPage < 1) {
        $this->itemCountPerPage = $this->getTotalItemCount();
    }
    $this->pageCount        = null;
    $this->currentItems     = null;
    $this->currentItemCount = null;

    return $this;
}

---

Comment

User: @Martin-P
Created On: 2015-04-30T08:03:32Z
Updated At: 2015-04-30T08:03:32Z
Body

Calling of Zend\Paginator\Paginator::setItemCountPerPage() calls immediately $this->getAdapter()->count().

Perhaps you can show where this happens? The current version of Zend\Paginator\Paginator::setItemCountPerPage() does not contain the code you are referring to.

$this->pageCount can not be set to null here, because the amount of pages changes at the moment the amount of items per page changes. When I have 10 items in total and have 10 items per page, it results in 1 page. When I have the same 10 items with 2 items per page, it results in 5 pages.

---

Comment

User: @fedys
Created On: 2015-04-30T08:15:06Z
Updated At: 2015-04-30T08:41:44Z
Body
https://github.com/zendframework/zf2/blob/8193bdcd4a38dc1b92ccd30cdb73c39fb4af4a90/library/Zend/Paginator/Paginator.php#L557 calls $this->_calculatePageCount() and it calls the $this->getAdapter()->count() in turn on line no. 874.
$this->pageCount = $this->_calculatePageCount(); on line no. 557 can be replaced by $this->pageCount = null; and thus call of $this->getAdapter()->count() can be postponed to time when it is really needed.

$this->pageCount can be null because it will be lazy-loaded in Zend\Paginator\Paginator::count() and this is the only place where $this->pageCount is used.

---

Comment

User: @Martin-P
Created On: 2015-04-30T08:54:03Z
Updated At: 2015-04-30T08:54:03Z
Body
Theoretically that is indeed possible. The problem is the class can be extended and people may depend on this variable to be set at that point. That makes this optimization a possible BC break.

---

Comment

User: @fedys
Created On: 2015-04-30T09:06:02Z
Updated At: 2015-04-30T09:06:02Z
Body
You are right, this could lead to BC break for those who extend this class.
Just another note :)

https://github.com/zendframework/zf2/blob/8193bdcd4a38dc1b92ccd30cdb73c39fb4af4a90/library/Zend/Paginator/Paginator.php#L341 can lead to multiple calls of $this->_calculatePageCount() if $this->_calculatePageCount() returns zero. The condition should be !isset($this->pageCount) instead of !$this->pageCount.

---

[samsonasik]

@fedys I've created PR #25 for check $this->pagecount in count() method.

[fedys]

@samsonasik I've created PR #26 containing testRepetitiveCallCountResultingZeroDoesNotCauseRepetitiveCallOfCalculatePageCountMethod() which really tests the repetitive call of Zend\Paginator\Paginator::_calculatePageCount() method