Merge pull request #2339 from edx/dhm/bug_perf_loc

Dhm/bug perf loc

cms/envs/aws.py

@@ -122,6 +122,13 @@ SITE_NAME = ENV_TOKENS['SITE_NAME']
 LOG_DIR = ENV_TOKENS['LOG_DIR']
 
 CACHES = ENV_TOKENS['CACHES']
+# Cache used for location mapping -- called many times with the same key/value
+# in a given request.
+if 'loc_cache' not in CACHES:
+    CACHES['loc_cache'] = {
+        'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
+        'LOCATION': 'edx_location_mem_cache',
+    }
 
 SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN')
 SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE)

common/djangoapps/student/roles.py

@@ -239,4 +239,4 @@ class CourseCreatorRole(GroupBasedRole):
     """
     ROLE = "course_creator_group"
     def __init__(self, *args, **kwargs):
-        super(CourseCreatorRole, self).__init__(self.ROLE, *args, **kwargs)
+        super(CourseCreatorRole, self).__init__([self.ROLE], *args, **kwargs)

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -256,7 +256,10 @@ class LocMapperStore(object):
 
         :param course_id: old style course id
         """
-        # doesn't use caching as cache requires full location w/o wildcards
+        cached = self._get_course_locator_from_cache(old_style_course_id, published)
+        if cached:
+            return cached
+
         location_id = self._interpret_location_course_id(old_style_course_id, location)
         if old_style_course_id is None:
             old_style_course_id = self._generate_location_course_id(location_id)
@@ -274,11 +277,13 @@ class LocMapperStore(object):
                 if 'name' not in item['_id']:
                     entry = item
                     break
+        published_course_locator = CourseLocator(package_id=entry['course_id'], branch=entry['prod_branch'])
+        draft_course_locator = CourseLocator(package_id=entry['course_id'], branch=entry['draft_branch'])
+        self._cache_course_locator(old_style_course_id, published_course_locator, draft_course_locator)
         if published:
-            branch = entry['prod_branch']
+            return published_course_locator
         else:
-            branch = entry['draft_branch']
-        return CourseLocator(package_id=entry['course_id'], branch=branch)
+            return draft_course_locator
 
     def _add_to_block_map(self, location, location_id, block_map):
         '''add the given location to the block_map and persist it'''
@@ -392,6 +397,17 @@ class LocMapperStore(object):
                 return entry[1]
         return None
 
+    def _get_course_locator_from_cache(self, old_course_id, published):
+        """
+        Get the course Locator for this old course id
+        """
+        entry = self.cache.get(old_course_id)
+        if entry is not None:
+            if published:
+                return entry[0].as_course_locator()
+            else:
+                return entry[1].as_course_locator()
+
     def _get_location_from_cache(self, locator):
         """
         See if the locator is in the cache. If so, return the mapped location.
@@ -405,6 +421,12 @@ class LocMapperStore(object):
         """
         return self.cache.get('courseId+{}'.format(locator_package_id))
 
+    def _cache_course_locator(self, old_course_id, published_course_locator, draft_course_locator):
+        """
+        For quick lookup of courses
+        """
+        self.cache.set(old_course_id, (published_course_locator, draft_course_locator))
+
     def _cache_location_map_entry(self, old_course_id, location, published_usage, draft_usage):
         """
         Cache the mapping from location to the draft and published Locators in entry.
@@ -417,4 +439,5 @@ class LocMapperStore(object):
         setmany[unicode(published_usage)] = location
         setmany[unicode(draft_usage)] = location
         setmany['{}+{}'.format(old_course_id, location.url())] = (published_usage, draft_usage)
+        setmany[old_course_id] = (published_usage, draft_usage)
         self.cache.set_many(setmany)

common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py

@@ -397,14 +397,20 @@ class TrivialCache(object):
     def __init__(self):
         self.cache = {}
 
-    def get(self, key):
+    def get(self, key, default=None):
         """
         Mock the .get
         """
-        return self.cache.get(key)
+        return self.cache.get(key, default)
 
     def set_many(self, entries):
         """
         mock set_many
         """
         self.cache.update(entries)
+
+    def set(self, key, entry):
+        """
+        mock set
+        """
+        self.cache[key] = entry

common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py

@@ -20,6 +20,7 @@ from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
 from xmodule.modulestore.mongo.base import MongoModuleStore
 from xmodule.modulestore.split_migrator import SplitMigrator
 from xmodule.modulestore.mongo import draft
+from xmodule.modulestore.tests import test_location_mapper
 
 
 class TestMigration(unittest.TestCase):
@@ -43,10 +44,8 @@ class TestMigration(unittest.TestCase):
 
     def setUp(self):
         super(TestMigration, self).setUp()
-        noop_cache = mock.Mock(spec=['get', 'set_many'])
-        noop_cache.configure_mock(**{'get.return_value': None})
         # pylint: disable=W0142
-        self.loc_mapper = LocMapperStore(noop_cache, **self.db_config)
+        self.loc_mapper = LocMapperStore(test_location_mapper.TrivialCache(), **self.db_config)
         self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options)
         self.draft_mongo = DraftModuleStore(self.db_config, **self.modulestore_options)
         self.split_mongo = SplitMongoModuleStore(

lms/envs/aws.py

@@ -155,6 +155,13 @@ MEDIA_URL = ENV_TOKENS['MEDIA_URL']
 LOG_DIR = ENV_TOKENS['LOG_DIR']
 
 CACHES = ENV_TOKENS['CACHES']
+# Cache used for location mapping -- called many times with the same key/value
+# in a given request.
+if 'loc_cache' not in CACHES:
+    CACHES['loc_cache'] = {
+        'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
+        'LOCATION': 'edx_location_mem_cache',
+    }
 
 # Email overrides
 DEFAULT_FROM_EMAIL = ENV_TOKENS.get('DEFAULT_FROM_EMAIL', DEFAULT_FROM_EMAIL)