From d34fa71df400463a6ad2feeebe077b9c70761894 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Fri, 19 Aug 2016 12:56:42 +0100 Subject: [PATCH] MDL-55628 Completion: Use simpledata for completion cache The completion cache is currently not marked as simpledata. On the course page it is frequently retrieved hundreds of times which results in many calls to the slow unserialize function. By making a slight change to the data format (using arrays instead of objects) we can mark it as simpledata, which will avoid using unserialize. --- lib/completionlib.php | 51 ++++++++++++++++---------------- lib/db/caches.php | 1 + lib/tests/completionlib_test.php | 6 ++-- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/completionlib.php b/lib/completionlib.php index de9b8a4e63988..cba3ed9707a23 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -894,15 +894,16 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null $usecache = $userid == $USER->id; $cacheddata = array(); if ($usecache) { + $key = $userid . '_' . $this->course->id; if (!isset($this->course->cacherev)) { $this->course = get_course($this->course_id); } - if ($cacheddata = $completioncache->get($userid . '_' . $this->course->id)) { + if ($cacheddata = $completioncache->get($key)) { if ($cacheddata['cacherev'] != $this->course->cacherev) { // Course structure has been changed since the last caching, forget the cache. $cacheddata = array(); - } else if (array_key_exists($cm->id, $cacheddata)) { - return $cacheddata[$cm->id]; + } else if (isset($cacheddata[$cm->id])) { + return (object)$cacheddata[$cm->id]; } } } @@ -921,10 +922,8 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null // Reindex by cm id $alldata = array(); - if ($alldatabycmc) { - foreach ($alldatabycmc as $data) { - $alldata[$data->coursemoduleid] = $data; - } + foreach ($alldatabycmc as $data) { + $alldata[$data->coursemoduleid] = (array)$data; } // Get the module info and build up condition info for each one @@ -932,17 +931,17 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null $modinfo = get_fast_modinfo($this->course, $userid); } foreach ($modinfo->cms as $othercm) { - if (array_key_exists($othercm->id, $alldata)) { + if (isset($alldata[$othercm->id])) { $data = $alldata[$othercm->id]; } else { // Row not present counts as 'not complete' - $data = new StdClass; - $data->id = 0; - $data->coursemoduleid = $othercm->id; - $data->userid = $userid; - $data->completionstate = 0; - $data->viewed = 0; - $data->timemodified = 0; + $data = array(); + $data['id'] = 0; + $data['coursemoduleid'] = $othercm->id; + $data['userid'] = $userid; + $data['completionstate'] = 0; + $data['viewed'] = 0; + $data['timemodified'] = 0; } $cacheddata[$othercm->id] = $data; } @@ -954,15 +953,17 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null } else { // Get single record $data = $DB->get_record('course_modules_completion', array('coursemoduleid'=>$cm->id, 'userid'=>$userid)); - if ($data == false) { + if ($data) { + $data = (array)$data; + } else { // Row not present counts as 'not complete' - $data = new StdClass; - $data->id = 0; - $data->coursemoduleid = $cm->id; - $data->userid = $userid; - $data->completionstate = 0; - $data->viewed = 0; - $data->timemodified = 0; + $data = array(); + $data['id'] = 0; + $data['coursemoduleid'] = $cm->id; + $data['userid'] = $userid; + $data['completionstate'] = 0; + $data['viewed'] = 0; + $data['timemodified'] = 0; } // Put in cache @@ -971,9 +972,9 @@ public function get_data($cm, $wholecourse = false, $userid = 0, $modinfo = null if ($usecache) { $cacheddata['cacherev'] = $this->course->cacherev; - $completioncache->set($userid . '_' . $this->course->id, $cacheddata); + $completioncache->set($key, $cacheddata); } - return $cacheddata[$cm->id]; + return (object)$cacheddata[$cm->id]; } /** diff --git a/lib/db/caches.php b/lib/db/caches.php index 7d90e55efebee..b2be0c0389682 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -221,6 +221,7 @@ 'completion' => array( 'mode' => cache_store::MODE_APPLICATION, 'simplekeys' => true, + 'simpledata' => true, 'ttl' => 3600, 'staticacceleration' => true, 'staticaccelerationsize' => 2, // Should be current course and site course. diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index d9a75de7af5df..f00447c7cadf9 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -418,7 +418,7 @@ public function test_get_data() { $result = $c->get_data($cm); $this->assertEquals($sillyrecord, $result); $cachevalue = $cache->get('314159_42'); - $this->assertEquals($sillyrecord, $cachevalue[13]); + $this->assertEquals((array)$sillyrecord, $cachevalue[13]); // 4. Current user, 'whole course', but from cache. $result = $c->get_data($cm, true); @@ -443,8 +443,8 @@ public function test_get_data() { // Check the cache contents. $cachevalue = $cache->get('314159_42'); - $this->assertEquals($basicrecord, $cachevalue[13]); - $this->assertEquals((object)array('id'=>'0', 'coursemoduleid'=>14, + $this->assertEquals($basicrecord, (object)$cachevalue[13]); + $this->assertEquals(array('id' => '0', 'coursemoduleid' => 14, 'userid'=>314159, 'completionstate'=>0, 'viewed'=>0, 'timemodified'=>0), $cachevalue[14]); }