From 2a54e091765bc0af9accf1a64941ccd5b23d6cbe Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Sat, 18 Jan 2025 23:13:06 -0500 Subject: [PATCH 1/2] perf: cache missing XBlock mappings in load_class Checking the entrypoints for the XBlock class we want for a given tag name is relatively expensive. We mitigate this by caching the mapping of tags to classes in Plugin.load_class(). But before this commit, we weren't caching failed lookups, i.e. the situation where there is no XBlock that maps to a given tag. This meant that if a course had many instances of a type of XBlock that is not installed on the given server, we would be constantly scanning entrypoints to look for our non-existent XBlock subclass. This really slows down things like the Studio course outline page. The fix here is to store failed lookups as None values in the PLUGIN_CACHE. --- xblock/plugin.py | 56 ++++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index 6b499946c..63f981dd7 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -142,28 +142,42 @@ def select(identifier, all_entry_points): """ identifier = identifier.lower() key = (cls.entry_point, identifier) - if key not in PLUGIN_CACHE: - if select is None: - select = default_select - - all_entry_points = [ - *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), - *importlib.metadata.entry_points(group=cls.entry_point, name=identifier) - ] - - for extra_identifier, extra_entry_point in iter(cls.extra_entry_points): - if identifier == extra_identifier: - all_entry_points.append(extra_entry_point) - - try: - selected_entry_point = select(identifier, all_entry_points) - except PluginMissingError: - if default is not None: - return default - raise - - PLUGIN_CACHE[key] = cls._load_class_entry_point(selected_entry_point) + if key in PLUGIN_CACHE: + xblock_cls = PLUGIN_CACHE[key] or default + if xblock_cls: + return xblock_cls + + # If the key was in PLUGIN_CACHE, but the value stored was None, and + # there was no default class to fall back to, we give up and raise + # PluginMissingError. This is for performance reasons. We've cached + # the fact that there is no XBlock class that maps to this + # identifier, and it is very expensive to check through the entry + # points each time. This assumes that XBlock class mappings do not + # change during the life of the process. + raise PluginMissingError(identifier) + + if select is None: + select = default_select + + all_entry_points = [ + *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), + *importlib.metadata.entry_points(group=cls.entry_point, name=identifier) + ] + + for extra_identifier, extra_entry_point in iter(cls.extra_entry_points): + if identifier == extra_identifier: + all_entry_points.append(extra_entry_point) + + try: + selected_entry_point = select(identifier, all_entry_points) + except PluginMissingError: + PLUGIN_CACHE[key] = None + if default is not None: + return default + raise + + PLUGIN_CACHE[key] = cls._load_class_entry_point(selected_entry_point) return PLUGIN_CACHE[key] From 24e4409f768a66a264f998a0bdf1ad043ac480fa Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Sat, 18 Jan 2025 23:29:01 -0500 Subject: [PATCH 2/2] chore: bump verison to v5.1.1 --- xblock/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xblock/__init__.py b/xblock/__init__.py index cd74464e7..bf676c4f5 100644 --- a/xblock/__init__.py +++ b/xblock/__init__.py @@ -2,4 +2,4 @@ XBlock Courseware Components """ -__version__ = '5.1.0' +__version__ = '5.1.1'