Bedrock performance tooling and initial optimisations (#15513)

* Add support for Django-silk profiling in local/non-prod only Also add script to hit some common/popular URLs to give djanjo-silk some traffic to capture * Add cacheing to geo.valid_country_code for performance boost Saves 11 SQL queries on the releasnotes page by cacheing the country code lookup for an hour. Tested on /en-US/firefox/132.0.1/releasenotes/ Cold cache: 14 queries / 2066ms Warm cache: 3 queries / 222ms * Rename 'default' cache time to CACHE_TIME_SHORT to make it more meaningful a name * Add cacheing to Careers views to reduce number of DB hits Caches derived values (location options, etc) for longer than the actual page of positions Takes careers listing page down from 9 queries to 2 for the CMS deployment and 0 on the Web deployment * Cache the newsletter lookup for 6 hours It's used on the newsletter form which shows up in a bunch of places on the site. * Add missing subdep to hashed requirements * Reinstate missing function call in _log() helper 🤦
mozilla · Jan 13, 2025 · 25b38f7 · 25b38f7
1 parent 9db97b8
commit 25b38f7
Show file tree

Hide file tree

Showing 12 changed files with 211 additions and 15 deletions.
diff --git a/bedrock/base/geo.py b/bedrock/base/geo.py
@@ -3,12 +3,20 @@
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 from django.conf import settings
+from django.core.cache import cache
 
 from product_details import product_details
 
 
 def valid_country_code(country):
-    codes = product_details.get_regions("en-US").keys()
+    _key = f"valid_country_codes_for_{country}"
+
+    codes = cache.get(_key)
+
+    if not codes:
+        codes = product_details.get_regions("en-US").keys()
+        cache.set(_key, codes, timeout=settings.CACHE_TIME_MED)
+
     if country and country.lower() in codes:
         return country.upper()
 

diff --git a/bedrock/careers/models.py b/bedrock/careers/models.py
@@ -5,6 +5,8 @@
 from datetime import datetime
 from itertools import chain
 
+from django.conf import settings
+from django.core.cache import cache
 from django.db import models
 from django.urls import reverse
 
@@ -37,28 +39,52 @@ def __str__(self):
     def get_absolute_url(self):
         return reverse("careers.position", kwargs={"source": self.source, "job_id": self.job_id})
 
+    @classmethod
+    def _get_cache_key(cls, name):
+        return f"careers_position__{name}"
+
     @property
     def location_list(self):
-        return sorted(self.location.split(","))
+        _key = self._get_cache_key("location_list")
+        location_list = cache.get(_key)
+        if location_list is None:
+            location_list = sorted(self.location.split(","))
+            cache.set(_key, location_list, settings.CACHE_TIME_LONG)
+        return location_list
 
     @classmethod
     def position_types(cls):
-        return sorted(set(cls.objects.values_list("position_type", flat=True)))
+        _key = cls._get_cache_key("position_types")
+        position_types = cache.get(_key)
+        if position_types is None:
+            position_types = sorted(set(cls.objects.values_list("position_type", flat=True)))
+            cache.set(_key, position_types, settings.CACHE_TIME_LONG)
+        return position_types
 
     @classmethod
     def locations(cls):
-        return sorted(
-            {
-                location.strip()
-                for location in chain(
-                    *[locations.split(",") for locations in cls.objects.exclude(job_locations="Remote").values_list("job_locations", flat=True)]
-                )
-            }
-        )
+        _key = cls._get_cache_key("locations")
+        locations = cache.get(_key)
+        if locations is None:
+            locations = sorted(
+                {
+                    location.strip()
+                    for location in chain(
+                        *[locations.split(",") for locations in cls.objects.exclude(job_locations="Remote").values_list("job_locations", flat=True)]
+                    )
+                }
+            )
+            cache.set(_key, locations, settings.CACHE_TIME_LONG)
+        return locations
 
     @classmethod
     def categories(cls):
-        return sorted(set(cls.objects.values_list("department", flat=True)))
+        _key = cls._get_cache_key("categories")
+        categories = cache.get(_key)
+        if categories is None:
+            categories = sorted(set(cls.objects.values_list("department", flat=True)))
+            cache.set(_key, categories, settings.CACHE_TIME_LONG)
+        return categories
 
     @property
     def cover(self):

diff --git a/bedrock/careers/tests/test_forms.py b/bedrock/careers/tests/test_forms.py
@@ -2,12 +2,18 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+
+from django.core.cache import cache
+
 from bedrock.careers.forms import PositionFilterForm
 from bedrock.careers.tests import PositionFactory
 from bedrock.mozorg.tests import TestCase
 
 
 class PositionFilterFormTests(TestCase):
+    def setUp(self):
+        cache.clear()
+
     def test_dynamic_position_type_choices(self):
         """
         The choices for the position_type field should be dynamically

diff --git a/bedrock/careers/tests/test_models.py b/bedrock/careers/tests/test_models.py
@@ -2,12 +2,17 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+from django.core.cache import cache
+
 from bedrock.careers.models import Position
 from bedrock.careers.tests import PositionFactory
 from bedrock.mozorg.tests import TestCase
 
 
 class TestPositionModel(TestCase):
+    def setUp(self):
+        cache.clear()
+
     def test_location_list(self):
         PositionFactory(location="San Francisco,Portland")
         pos = Position.objects.get()

diff --git a/bedrock/careers/tests/test_utils.py b/bedrock/careers/tests/test_utils.py
@@ -2,13 +2,16 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+from django.core.cache import cache
+
 from bedrock.careers.tests import PositionFactory
 from bedrock.careers.utils import generate_position_meta_description
 from bedrock.mozorg.tests import TestCase
 
 
 class GeneratePositionMetaDescriptionTests(TestCase):
     def setUp(self):
+        cache.clear()
         self.position = PositionFactory(title="Bowler", position_type="Full time", location="Los Angeles,Ralphs")
 
     def test_position_type_consonant_beginning(self):

diff --git a/bedrock/careers/views.py b/bedrock/careers/views.py
@@ -2,6 +2,8 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+from django.conf import settings
+from django.core.cache import cache
 from django.http.response import Http404
 from django.shortcuts import get_object_or_404
 from django.views.generic import DetailView, ListView
@@ -62,10 +64,17 @@ class BenefitsView(L10nTemplateView):
 
 
 class PositionListView(LangFilesMixin, RequireSafeMixin, ListView):
-    queryset = Position.objects.exclude(job_locations="Remote")
     template_name = "careers/listings.html"
     context_object_name = "positions"
 
+    def get_queryset(self):
+        _key = "careers_position_listing_qs"
+        qs = cache.get(_key)
+        if qs is None:
+            qs = Position.objects.exclude(job_locations="Remote")
+            cache.set(_key, qs, settings.CACHE_TIME_SHORT)
+        return qs
+
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         context["form"] = PositionFilterForm()

diff --git a/bedrock/newsletter/utils.py b/bedrock/newsletter/utils.py
@@ -2,6 +2,9 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+from django.conf import settings
+from django.core.cache import cache
+
 import basket
 
 from bedrock.newsletter.models import Newsletter
@@ -12,7 +15,12 @@ def get_newsletters():
     Keys are the internal keys we use to designate newsletters to basket.
     Values are dictionaries with the remaining newsletter information.
     """
-    return Newsletter.objects.serialize()
+    _key = "serialized_newsletters"
+    serialized_newsletters = cache.get(_key)
+    if serialized_newsletters is None:
+        serialized_newsletters = Newsletter.objects.serialize()
+        cache.set(_key, serialized_newsletters, timeout=settings.CACHE_TIME_LONG)
+    return serialized_newsletters
 
 
 def get_languages_for_newsletters(newsletters=None):

diff --git a/bedrock/settings/base.py b/bedrock/settings/base.py
@@ -86,11 +86,16 @@ def data_path(*args):
         "image_renditions": {"URL": f"{REDIS_URL}/0"},
     }
 
+CACHE_TIME_SHORT = 60 * 10  # 10 mins
+CACHE_TIME_MED = 60 * 60  # 1 hour
+CACHE_TIME_LONG = 60 * 60 * 6  # 6 hours
+
+
 CACHES = {
     "default": {
         "BACKEND": "bedrock.base.cache.SimpleDictCache",
         "LOCATION": "default",
-        "TIMEOUT": 600,
+        "TIMEOUT": CACHE_TIME_SHORT,
         "OPTIONS": {
             "MAX_ENTRIES": 5000,
             "CULL_FREQUENCY": 4,  # 1/4 entries deleted if max reached
@@ -2443,3 +2448,11 @@ def lazy_wagtail_langs():
     # Useful when customising the Wagtail admin
     # when enabled, will be visible on cms-admin/styleguide
     INSTALLED_APPS.append("wagtail.contrib.styleguide")
+
+# Django-silk for performance profiling
+if ENABLE_DJANGO_SILK := config("ENABLE_DJANGO_SILK", default="False", parser=bool):
+    print("Django-Silk profiling enabled - go to http://localhost:8000/silk/ to view metrics")
+    INSTALLED_APPS.append("silk")
+    MIDDLEWARE.insert(0, "silk.middleware.SilkyMiddleware")
+    SUPPORTED_NONLOCALES.append("silk")
+    SILKY_PYTHON_PROFILER = config("SILKY_PYTHON_PROFILER", default="False", parser=bool)
diff --git a/bedrock/urls.py b/bedrock/urls.py
@@ -72,6 +72,9 @@
         path("_internal_draft_preview/", include(wagtaildraftsharing_urls)),  # ONLY available in CMS mode
     )
 
+if settings.ENABLE_DJANGO_SILK:
+    urlpatterns += [path("silk/", include("silk.urls", namespace="silk"))]
+
 if settings.DEFAULT_FILE_STORAGE == "django.core.files.storage.FileSystemStorage":
     # Serve media files from Django itself - production won't use this
     from django.urls import re_path

diff --git a/profiling/hit_popular_pages.py b/profiling/hit_popular_pages.py
@@ -0,0 +1,96 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+"""Request a selection of pages that are populat on www.m.o from your local
+runserver, so  that django-silk can capture performance info on them.
+
+Usage:
+
+    1. In your .env set ENABLE_DJANGO_SILK=True
+    2. Start your runserver on port 8000
+    3. python profiling/hit_popular_pages.py
+    3. View results at http://localhost:8000/silk/
+
+"""
+
+import sys
+import time
+
+import requests
+
+paths = [
+    "/en-US/firefox/126.0/whatsnew/",
+    "/en-US/firefox/",
+    "/en-US/firefox/windows/",
+    "/en-US/firefox/new/?reason=manual-update",
+    "/en-US/firefox/download/thanks/",
+    "/en-US/firefox/new/?reason=outdated",
+    "/en-US/firefox/features/",
+    "/en-US/firefox/all/",
+    "/en-US/firefox/welcome/18/",
+    "/en-US/",
+    "/en-US/firefox/installer-help/?channel=release&installer_lang=en-US",
+    "/en-US/firefox/download/thanks/?s=direct",
+    "/en-US/firefox/welcome/19/",
+    "/en-US/firefox/enterprise/?reason=manual-update",
+    "/en-US/products/vpn/",
+    "/en-US/firefox/browsers/windows-64-bit/",
+    "/en-US/firefox/mac/",
+    "/en-US/about/",
+    "/en-US/firefox/android/124.0/releasenotes/",
+    "/en-US/firefox/browsers/mobile/get-app/",
+    "/en-US/firefox/browsers/",
+    "/en-US/firefox/nightly/firstrun/",
+    "/en-US/firefox/developer/",
+    "/en-US/account/",
+    "/en-US/contribute/",
+    "/en-US/firefox/browsers/mobile/android/",
+    "/en-US/privacy/archive/firefox-fire-tv/2023-06/",
+    "/en-US/firefox/121.0/system-requirements/",
+    "/en-US/firefox/browsers/mobile/",
+    "/en-US/firefox/releases/",
+    "/en-US/MPL/",
+    "/en-US/firefox/enterprise/",
+    "/en-US/security/advisories/",
+    "/en-US/firefox/browsers/what-is-a-browser/",
+    "/en-US/firefox/channel/desktop/?reason=manual-update",
+    "/en-US/firefox/pocket/",
+    "/en-US/firefox/channel/desktop/",
+    "/en-US/firefox/welcome/17b/",
+    "/en-US/firefox/welcome/17c/",
+    "/en-US/firefox/welcome/17a/",
+    "/en-US/firefox/set-as-default/thanks/",
+    "/en-US/careers/listings/",
+    "/en-US/firefox/browsers/chromebook/",
+    "/en-US/firefox/nothing-personal/",
+    "/en-US/newsletter/existing/",
+    "/en-US/about/legal/terms/firefox/",
+    "/en-US/firefox/linux/",
+    "/en-US/firefox/browsers/mobile/focus/",
+    "/en-US/products/vpn/download/",
+    "/en-US/about/manifesto/",
+    "/en-US/stories/joy-of-color/",
+    "/en-US/contact/",
+    "/en-US/about/legal/defend-mozilla-trademarks/",
+]
+
+
+def _log(*args):
+    sys.stdout.write("\n".join(args))
+
+
+def hit_pages(paths, times=3):
+    _base_url = "http://localhost:8000"
+
+    for path in paths:
+        for _ in range(times):
+            time.sleep(0.5)
+            url = f"{_base_url}{path}"
+            requests.get(url)
+
+    _log("All done")
+
+
+if __name__ == "__main__":
+    hit_pages(paths)
diff --git a/requirements/dev.in b/requirements/dev.in
@@ -2,6 +2,7 @@
 
 bpython==0.24
 braceexpand==0.1.7
+django-silk==5.3.1
 factory-boy==3.3.1
 freezegun==1.5.1
 markdown-it-py>=2.2.0