Merge pull request #200 from seddonym/fix-cache-benchmarking-issues

Address issue with setting up the partial cache in benchmark

Author: seddonym

tests/benchmarking/adaptors.py

@@ -0,0 +1,20 @@
+from grimp.adaptors.caching import Cache, CacheMiss
+from typing import Set
+
+from grimp.application.ports.modulefinder import ModuleFile
+from grimp.domain.valueobjects import DirectImport
+
+
+class PrefixMissingCache(Cache):
+    """
+    Test double of the real cache that will miss caching any module that begins with
+    a special prefix.
+    """
+
+    MISSING_PREFIX = "miss_marker_8772f06d64b6_"  # Arbitrary prefix.
+
+    def read_imports(self, module_file: ModuleFile) -> Set[DirectImport]:
+        leaf_name = module_file.module.name.split(".")[-1]
+        if leaf_name.startswith(self.MISSING_PREFIX):
+            raise CacheMiss
+        return super().read_imports(module_file)

tests/benchmarking/test_benchmarking.py

@@ -2,10 +2,13 @@
 import json
 import importlib
 from pathlib import Path
+
+from tests.config import override_settings
 from grimp.adaptors.graph import ImportGraph
 from grimp import PackageDependency, Route
 import grimp
 from copy import deepcopy
+from .adaptors import PrefixMissingCache
 
 
 def _run_benchmark(benchmark, fn, *args, **kwargs):
@@ -317,6 +320,7 @@ def test_build_django_from_cache_no_misses(benchmark):
     (
         2,  # Fewer than the likely number of CPUs.
         15,  # A bit more than the likely number of CPUs.
+        350,  # Around half the Django codebase.
     ),
 )
 def test_build_django_from_cache_a_few_misses(benchmark, number_of_misses):
@@ -325,15 +329,30 @@ def test_build_django_from_cache_a_few_misses(benchmark, number_of_misses):
 
     This benchmark utilizes the cache except for a few modules, which we add.
     """
-    # Populate the cache first, before beginning the benchmark.
-    grimp.build_graph("django")
-    # Add a module which won't be in the cache.
-    django_path = Path(importlib.util.find_spec("django").origin).parent
-    for i in range(number_of_misses):
-        new_module = django_path / f"new_module_{i}.py"
-        new_module.write_text("from django.db import models")
+    # We must use a special cache class, otherwise the cache will be populated
+    # by the first iteration. It would be better to do this using a setup function,
+    # which is supported by pytest-benchmark's pedantic mode, but not codspeed.
+    # This won't give us a truly accurate picture, but it's better than nothing.
 
-    _run_benchmark(benchmark, grimp.build_graph, "django")
+    # Add some specially-named modules which will be treated as not in the cache.
+    django_path = Path(importlib.util.find_spec("django").origin).parent
+    extra_modules = [
+        django_path / f"{PrefixMissingCache.MISSING_PREFIX}{i}.py" for i in range(number_of_misses)
+    ]
+    # Use some real python, which will take time to parse.
+    module_to_copy = django_path / "forms" / "forms.py"
+    module_contents = module_to_copy.read_text()
+    for extra_module in extra_modules:
+        extra_module.write_text(module_contents)
+
+    with override_settings(CACHE_CLASS=PrefixMissingCache):
+        # Populate the cache.
+        grimp.build_graph("django")
+
+        _run_benchmark(benchmark, grimp.build_graph, "django")
+
+    # Clean up.
+    [module.unlink() for module in extra_modules]
 
 
 class TestFindIllegalDependenciesForLayers: