From c28c192ef7fa57e95f0b3522341ddf01d3239760 Mon Sep 17 00:00:00 2001 From: Lin Guo Date: Fri, 31 Jul 2026 10:04:29 -0700 Subject: [PATCH] Replace manual cache with functools lru-cache This has the benefit of restricting unbounded cache growth, and improves on readability, without having the manual cache implementation cluttering the main logic. --- lib/ramble/ramble/expander.py | 14 +++------- .../ramble/language/language_helpers.py | 2 +- lib/ramble/ramble/repository.py | 27 +++++++++---------- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/lib/ramble/ramble/expander.py b/lib/ramble/ramble/expander.py index 6d3c7ea01..4b1526ac6 100644 --- a/lib/ramble/ramble/expander.py +++ b/lib/ramble/ramble/expander.py @@ -26,7 +26,6 @@ from ramble.util.logger import logger from ramble.util.path import substitute_config_variables -_ast_cache: Dict[str, str] = {} # Regex for detecting math operators or keywords # We check for: + - * / % ^ & | ~ < > = ( ) [ ] { } , ' " # And keywords: and, or, in, is, not @@ -92,18 +91,13 @@ def _get_source_segment(source, node): return None +@functools.lru_cache(maxsize=2048) def _ast_parse(in_str): - """Parse a string into an AST, with caching.""" - if in_str in _ast_cache: - return _ast_cache[in_str] - + """Parse a string into an AST, with LRU caching.""" try: - math_ast = ast.parse(in_str, mode="eval") + return ast.parse(in_str, mode="eval") except SyntaxError: - math_ast = None - - _ast_cache[in_str] = math_ast - return math_ast + return None def _and(a, b): diff --git a/lib/ramble/ramble/language/language_helpers.py b/lib/ramble/ramble/language/language_helpers.py index cd5938591..a54d0e3eb 100644 --- a/lib/ramble/ramble/language/language_helpers.py +++ b/lib/ramble/ramble/language/language_helpers.py @@ -414,7 +414,7 @@ def is_specifier_set_compatible(spec_set): return True -@functools.lru_cache(maxsize=None) +@functools.lru_cache(maxsize=4096) def _parse_when(w_set): from ramble.util.format import when_order diff --git a/lib/ramble/ramble/repository.py b/lib/ramble/ramble/repository.py index 4ed70f600..c42782e84 100644 --- a/lib/ramble/ramble/repository.py +++ b/lib/ramble/ramble/repository.py @@ -470,23 +470,20 @@ class FastObjectChecker(Mapping): during instance initialization. """ - #: Global cache, reused by every instance - _paths_cache: Mapping[str, str] = {} - def __init__(self, objects_path, object_file_name, object_type): # The path of the repository managed by this instance self.objects_path = objects_path self.object_file_name = object_file_name self.object_type = object_type - # If the cache we need is not there yet, then build it appropriately - if objects_path not in self._paths_cache: - self._paths_cache[objects_path] = self._create_new_cache() - #: Reference to the appropriate entry in the global cache - self._objects_to_stats = self._paths_cache[objects_path] + self._objects_to_stats = self._create_new_cache( + objects_path, object_file_name, object_type + ) - def _create_new_cache(self): + @staticmethod + @functools.lru_cache() + def _create_new_cache(objects_path, object_file_name, object_type): """Create a new cache for objects in a repo. The implementation here should try to minimize filesystem @@ -497,24 +494,24 @@ def _create_new_cache(self): # Create a dictionary that will store the mapping between a # object name and its stat info cache = {} - if not os.path.isdir(self.objects_path): + if not os.path.isdir(objects_path): return cache - for obj_name in os.listdir(self.objects_path): + for obj_name in os.listdir(objects_path): # Skip non-directories in the object root. - obj_dir = os.path.join(self.objects_path, obj_name) + obj_dir = os.path.join(objects_path, obj_name) # Warn about invalid names that look like objects. if not nm.valid_module_name(obj_name): if not obj_name.startswith(".") and obj_name not in _ALL_ACCEPTED_CONFIGS: logger.warn( - f"Skipping {self.object_type} " + f"Skipping {object_type} " f'at {obj_dir}. "{obj_name}" is not ' "a valid Ramble module name." ) continue # Construct the file name from the directory - obj_file = os.path.join(self.objects_path, obj_name, self.object_file_name) + obj_file = os.path.join(objects_path, obj_name, object_file_name) # Use stat here to avoid lots of calls to the filesystem. try: @@ -524,7 +521,7 @@ def _create_new_cache(self): # No application.py file here. continue elif e.errno == errno.EACCES: - logger.warn(f"Can't read {self.object_type} file {obj_file}.") + logger.warn(f"Can't read {object_type} file {obj_file}.") continue raise e