? Success

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
4 Oct 2016

Pull Request for Issue #12133 and other optimizations.

Summary of Changes

  • Deprecate get and store methods in JCacheController, subclasses should have own methods.
  • Deprecate start and end methods in JCacheControllerOutput - joomla does not use it
  • Deprecate call method in JCacheControllerCallback
  • Remove setLifeTime, setCaching from JCacheController because __call() method can call it directly on JCache instance.
  • Replace operator == to === for locked and locklooped variables.
  • Do not store if lock can not be locked:
if ($locktest->locked === false && $locktest->locklooped === true)
  • Add get and store method to JCacheControllerOutput which is the same as deprecated method get and store in JCacheController.
  • In subclasses of JCacheController I did optimization on get methods.
    • move unlock() method before unserialize(...)
    • Generally unlock cache as fast as we can
  • Do not use ternary operator on variables with a lots of data, see http://fabien.potencier.org/the-php-ternary-operator-fast-or-not.html

Testing Instructions

Joomla should work as before.
Code review.

Documentation Changes Required

None.

avatar csthomas csthomas - open - 4 Oct 2016
avatar csthomas csthomas - change - 4 Oct 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Oct 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 4 Oct 2016
Category Libraries
avatar csthomas csthomas - change - 5 Oct 2016
The description was changed
avatar csthomas csthomas - edited - 5 Oct 2016
avatar csthomas csthomas - change - 28 Oct 2016
The description was changed
avatar csthomas csthomas - edited - 28 Oct 2016
avatar joomla-cms-bot joomla-cms-bot - change - 28 Oct 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 29 Oct 2016
Labels Removed: ?
avatar csthomas csthomas - change - 9 Dec 2016
The description was changed
avatar csthomas csthomas - change - 10 Dec 2016
The description was changed
avatar csthomas csthomas - edited - 10 Dec 2016
avatar csthomas csthomas - change - 10 Dec 2016
The description was changed
avatar csthomas csthomas - edited - 10 Dec 2016
avatar csthomas csthomas - change - 10 Dec 2016
The description was changed
avatar csthomas csthomas - edited - 10 Dec 2016
avatar csthomas
csthomas - comment - 10 Dec 2016

Can I ask you @mbabker for a review?

avatar csthomas
csthomas - comment - 10 Dec 2016

Thanks Michael.

Now I have to wait for 2 successful tests:)

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Jan 2017

@csthomas Enable Cache and test if J works as before?

avatar csthomas
csthomas - comment - 11 Jan 2017

Yes, be sure that cache works before patch

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 12 Jan 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 12 Jan 2017

I have tested this item successfully on 87f42d5

Tested on Articles, Blog, List, Contact, Feed, Search.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12312.

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 12 Jan 2017 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 27 Jan 2017

I have tested this item successfully on 87f42d5

Logged in, logged out, created article, viewed list, viewed article, changed site settings on frontend, ect. Seems to work great on every view I tested. Successfully tested this PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12312.

avatar JoshuaLewis JoshuaLewis - test_item - 27 Jan 2017 - Tested successfully
avatar jeckodevelopment jeckodevelopment - change - 27 Jan 2017
Status Pending Ready to Commit
avatar jeckodevelopment
jeckodevelopment - comment - 27 Jan 2017

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12312.

avatar rdeutz rdeutz - change - 27 Jan 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-01-27 18:18:10
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 27 Jan 2017
avatar rdeutz rdeutz - merge - 27 Jan 2017

Add a Comment

Login with GitHub to post a comment