diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 5a47a1aa73..d9123dedc8 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -16,6 +16,7 @@ from django.core.cache import caches from django.test.client import RequestFactory from django.test.utils import override_settings from nose.plugins.attrib import attr +from opaque_keys.edx.keys import CourseKey from pytz import UTC from request_cache.middleware import RequestCache from student.models import CourseEnrollment @@ -23,7 +24,7 @@ from student.tests.factories import UserFactory from xblock.core import XBlock from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \ TEST_DATA_SPLIT_MODULESTORE, TEST_DATA_MONGO_MODULESTORE -from xmodule.modulestore.tests.factories import check_mongo_calls_range, CourseFactory, check_sum_of_calls +from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, check_sum_of_calls from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin from ccx_keys.locator import CCXLocator from lms.djangoapps.ccx.tests.factories import CcxFactory @@ -125,15 +126,12 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT self.student, course_key ) + return CourseKey.from_string(unicode(course_key)) - def grade_course(self, course, view_as_ccx): + def grade_course(self, course_key): """ Renders the progress page for the given course. """ - course_key = course.id - if view_as_ccx: - course_key = CCXLocator.from_course_locator(course_key, self.ccx.id) - return progress( self.request, course_id=unicode(course_key), @@ -145,7 +143,7 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT Assert that mongodb is queried ``calls`` times in the surrounded context. """ - return check_mongo_calls_range(max_finds=calls) + return check_mongo_calls(calls) def assertXBlockInstantiations(self, instantiations): """ @@ -156,12 +154,12 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT def instrument_course_progress_render( self, course_width, enable_ccx, view_as_ccx, - default_queries, history_queries, reads, xblocks + sql_queries, mongo_reads, ): """ Renders the progress page, instrumenting Mongo reads and SQL queries. """ - self.setup_course(course_width, enable_ccx, view_as_ccx) + course_key = self.setup_course(course_width, enable_ccx, view_as_ccx) # Switch to published-only mode to simulate the LMS with self.settings(MODULESTORE_BRANCH='published-only'): @@ -170,7 +168,7 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT caches[cache].clear() # Refill the metadata inheritance cache - get_course_in_cache(self.course.id) + get_course_in_cache(course_key) # We clear the request cache to simulate a new request in the LMS. RequestCache.clear_request_cache() @@ -179,11 +177,11 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT # can actually take affect. OverrideFieldData.provider_classes = None - with self.assertNumQueries(default_queries, using='default'): - with self.assertNumQueries(history_queries, using='student_module_history'): - with self.assertMongoCallCount(reads): - with self.assertXBlockInstantiations(xblocks): - self.grade_course(self.course, view_as_ccx) + with self.assertNumQueries(sql_queries, using='default'): + with self.assertNumQueries(0, using='student_module_history'): + with self.assertMongoCallCount(mongo_reads): + with self.assertXBlockInstantiations(1): + self.grade_course(course_key) @ddt.data(*itertools.product(('no_overrides', 'ccx'), range(1, 4), (True, False), (True, False))) @ddt.unpack @@ -212,11 +210,11 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT XBLOCK_FIELD_DATA_WRAPPERS=['lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap'], MODULESTORE_FIELD_OVERRIDE_PROVIDERS=providers[overrides], ): - default_queries, history_queries, reads, xblocks = self.TEST_DATA[ + sql_queries, mongo_reads = self.TEST_DATA[ (overrides, course_width, enable_ccx, view_as_ccx) ] self.instrument_course_progress_render( - course_width, enable_ccx, view_as_ccx, default_queries, history_queries, reads, xblocks + course_width, enable_ccx, view_as_ccx, sql_queries, mongo_reads, ) @@ -230,28 +228,20 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): TEST_DATA = { # (providers, course_width, enable_ccx, view_as_ccx): ( # # of sql queries to default, - # # sql queries to student_module_history, # # of mongo queries, - # # of xblocks # ) - ('no_overrides', 1, True, False): (34, 0, 6, 1), - ('no_overrides', 2, True, False): (40, 0, 6, 1), - ('no_overrides', 3, True, False): (50, 0, 6, 1), - ('ccx', 1, True, False): (34, 0, 6, 1), - ('ccx', 2, True, False): (40, 0, 6, 1), - ('ccx', 3, True, False): (50, 0, 6, 1), - ('ccx', 1, True, True): (47, 0, 6, 1), - ('ccx', 2, True, True): (40, 0, 6, 1), - ('ccx', 3, True, True): (50, 0, 6, 1), - ('no_overrides', 1, False, False): (34, 0, 6, 1), - ('no_overrides', 2, False, False): (40, 0, 6, 1), - ('no_overrides', 3, False, False): (50, 0, 6, 1), - ('ccx', 1, False, False): (34, 0, 6, 1), - ('ccx', 2, False, False): (40, 0, 6, 1), - ('ccx', 3, False, False): (50, 0, 6, 1), - ('ccx', 1, False, True): (47, 0, 6, 1), - ('ccx', 2, False, True): (40, 0, 6, 1), - ('ccx', 3, False, True): (50, 0, 6, 1), + ('no_overrides', 1, True, False): (34, 6), + ('no_overrides', 2, True, False): (40, 6), + ('no_overrides', 3, True, False): (50, 6), + ('ccx', 1, True, False): (34, 6), + ('ccx', 2, True, False): (40, 6), + ('ccx', 3, True, False): (50, 6), + ('no_overrides', 1, False, False): (34, 6), + ('no_overrides', 2, False, False): (40, 6), + ('no_overrides', 3, False, False): (50, 6), + ('ccx', 1, False, False): (34, 6), + ('ccx', 2, False, False): (40, 6), + ('ccx', 3, False, False): (50, 6), } @@ -263,22 +253,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (34, 0, 4, 1), - ('no_overrides', 2, True, False): (40, 0, 19, 1), - ('no_overrides', 3, True, False): (50, 0, 84, 1), - ('ccx', 1, True, False): (34, 0, 4, 1), - ('ccx', 2, True, False): (40, 0, 19, 1), - ('ccx', 3, True, False): (50, 0, 84, 1), - ('ccx', 1, True, True): (35, 0, 5, 6), - ('ccx', 2, True, True): (41, 0, 20, 47), - ('ccx', 3, True, True): (51, 0, 85, 202), - ('no_overrides', 1, False, False): (34, 0, 4, 1), - ('no_overrides', 2, False, False): (40, 0, 19, 1), - ('no_overrides', 3, False, False): (50, 0, 84, 1), - ('ccx', 1, False, False): (34, 0, 4, 1), - ('ccx', 2, False, False): (40, 0, 19, 1), - ('ccx', 3, False, False): (50, 0, 84, 1), - ('ccx', 1, False, True): (46, 0, 4, 1), - ('ccx', 2, False, True): (118, 0, 19, 1), - ('ccx', 3, False, True): (398, 0, 84, 1), + ('no_overrides', 1, True, False): (34, 3), + ('no_overrides', 2, True, False): (40, 3), + ('no_overrides', 3, True, False): (50, 3), + ('ccx', 1, True, False): (34, 3), + ('ccx', 2, True, False): (40, 3), + ('ccx', 3, True, False): (50, 3), + ('ccx', 1, True, True): (35, 3), + ('ccx', 2, True, True): (41, 3), + ('ccx', 3, True, True): (51, 3), + ('no_overrides', 1, False, False): (34, 3), + ('no_overrides', 2, False, False): (40, 3), + ('no_overrides', 3, False, False): (50, 3), + ('ccx', 1, False, False): (34, 3), + ('ccx', 2, False, False): (40, 3), + ('ccx', 3, False, False): (50, 3), } diff --git a/openedx/core/lib/block_structure/block_structure.py b/openedx/core/lib/block_structure/block_structure.py index 7bc88cd0b6..fea54061ad 100644 --- a/openedx/core/lib/block_structure/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -8,7 +8,6 @@ The following internal data structures are implemented: _BlockRelations - Data structure for a single block's relations. _BlockData - Data structure for a single block's data. """ -from collections import defaultdict from logging import getLogger from openedx.core.lib.graph_traversals import traverse_topologically, traverse_post_order @@ -58,8 +57,8 @@ class BlockStructure(object): # Map of a block's usage key to its block relations. The # existence of a block in the structure is determined by its # presence in this map. - # defaultdict {UsageKey: _BlockRelations} - self._block_relations = defaultdict(_BlockRelations) + # dict {UsageKey: _BlockRelations} + self._block_relations = {} # Add the root block. self._add_block(self._block_relations, root_block_usage_key) @@ -207,7 +206,7 @@ class BlockStructure(object): # Create a new block relations map to store only those blocks # that are still linked - pruned_block_relations = defaultdict(_BlockRelations) + pruned_block_relations = {} old_block_relations = self._block_relations # Build the structure from the leaves up by doing a post-order @@ -245,7 +244,7 @@ class BlockStructure(object): relations map. Arguments: - block_relations (defaultdict({UsageKey: _BlockRelations})) - + block_relations (dict({UsageKey: _BlockRelations})) - Internal map of a block's usage key to its parents/children relations. @@ -253,6 +252,9 @@ class BlockStructure(object): child_key (UsageKey) - Usage key of the child block. """ + BlockStructure._add_block(block_relations, parent_key) + BlockStructure._add_block(block_relations, child_key) + block_relations[child_key].parents.append(parent_key) block_relations[parent_key].children.append(child_key) @@ -262,14 +264,15 @@ class BlockStructure(object): Adds the given usage_key to the given block_relations map. Arguments: - block_relations (defaultdict({UsageKey: _BlockRelations})) - + block_relations (dict({UsageKey: _BlockRelations})) - Internal map of a block's usage key to its parents/children relations. usage_key (UsageKey) - Usage key of the block that is to be added to the given block_relations. """ - block_relations[usage_key] = _BlockRelations() + if usage_key not in block_relations: + block_relations[usage_key] = _BlockRelations() class FieldData(object):