From 5c2ef08fa3b7a38c4b1498b6212728ab2e4c11ed 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 de9b8a4e639..cba3ed9707a 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -894,15 +894,16 @@ class completion_info { $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 @@ class completion_info { // 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 @@ class completion_info { $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 @@ class completion_info { } 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 @@ class completion_info { 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 912fe5aa611..da6b531c747 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -223,6 +223,7 @@ $definitions = array( '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 ee008efa4c5..da9265f1692 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -428,7 +428,7 @@ class core_completionlib_testcase extends advanced_testcase { $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); @@ -453,8 +453,8 @@ class core_completionlib_testcase extends advanced_testcase { // 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]); }