Skip to content

Commit

Permalink
MDL-55628 Completion: Use simpledata for completion cache
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sammarshallou committed Aug 22, 2016
1 parent 490e8f9 commit d34fa71
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 28 deletions.
51 changes: 26 additions & 25 deletions lib/completionlib.php
Expand Up @@ -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];
}
}
}
Expand All @@ -921,28 +922,26 @@ 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
if (empty($modinfo)) {
$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;
}
Expand All @@ -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
Expand All @@ -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];
}

/**
Expand Down
1 change: 1 addition & 0 deletions lib/db/caches.php
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions lib/tests/completionlib_test.php
Expand Up @@ -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);
Expand All @@ -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]);
}
Expand Down

0 comments on commit d34fa71

Please sign in to comment.