Optimize rating initialization

server/games/game.py

@@ -861,17 +861,22 @@ def report_army_stats(self, stats_json):
         self._process_pending_army_stats()
 
     def is_visible_to_player(self, player: Player) -> bool:
+        """
+        Determine if a player should see this game in their games list.
+
+        Note: This is a *hot* function, it can have significant impacts on
+        performance.
+        """
         if self.host is None:
             return False
 
         if player == self.host or player in self._connections:
             return True
 
-        mean, dev = player.ratings[self.rating_type]
-        displayed_rating = mean - 3 * dev
         if (
             self.enforce_rating_range
-            and displayed_rating not in self.displayed_rating_range
+            and player.get_displayed_rating(self.rating_type)
+            not in self.displayed_rating_range
         ):
             return False
 

server/players.py

@@ -88,6 +88,10 @@ def faction(self, value: Union[str, int, Faction]) -> None:
         else:
             self._faction = Faction.from_value(value)
 
+    def get_displayed_rating(self, rating_type: str) -> float:
+        mean, dev = self.ratings[rating_type]
+        return mean - 3 * dev
+
     def power(self) -> int:
         """An artifact of the old permission system. The client still uses this
         number to determine if a player gets a special category in the user list

server/rating.py

@@ -56,6 +56,8 @@ def __init__(self, leaderboards: Dict[str, Leaderboard], init: bool = True):
         self.leaderboards = leaderboards
         # Rating types which are present but should be recomputed.
         self.transient: Set[str] = set()
+        # Rating types which have been computed since the last rating update
+        self.clean: Set[str] = set()
 
         # DEPRECATED: Initialize known rating types so the client can display them
         if init:
@@ -73,6 +75,11 @@ def __setitem__(
             rating = value
 
         self.transient.discard(rating_type)
+        # This could be optimized further by walking backwards along the
+        # initialization chain and only unmarking the ratings we come accross,
+        # but this adds complexity so we won't bother unless it really becomes
+        # a performance bottleneck, which is unlikely.
+        self.clean.clear()
         super().__setitem__(rating_type, rating)
 
     def __getitem__(
@@ -83,14 +90,18 @@ def __getitem__(
         history = history or set()
         entry = self.get(rating_type)
 
-        if entry is None or rating_type in self.transient:
+        if (
+            entry is None or
+            (rating_type not in self.clean and rating_type in self.transient)
+        ):
             # Check for cycles
             if rating_type in history:
                 return default_rating()
 
             rating = self._get_initial_rating(rating_type, history=history)
 
             self.transient.add(rating_type)
+            self.clean.add(rating_type)
             super().__setitem__(rating_type, rating)
             return rating
 
@@ -117,6 +128,7 @@ def _get_initial_rating(
 
     def update(self, other: Dict[str, Rating]):
         self.transient -= set(other)
+        self.clean.clear()
         if isinstance(other, PlayerRatings):
             self.transient |= other.transient
         super().update(other)

tests/unit_tests/test_rating.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 import pytest
 
 from server.rating import Leaderboard, PlayerRatings, RatingType
@@ -135,6 +137,16 @@ def test_initialization_transient(chained_ratings):
     assert chained_ratings["tmm_2v2"] == (300, 200)
 
 
+def test_initialization_transient_caching(chained_ratings):
+    # Return a new mock on each call. By default it will return the same mock
+    chained_ratings._get_initial_rating = mock.Mock(side_effect=mock.Mock)
+
+    chained_ratings["global"] = (1000, 50)
+
+    assert chained_ratings["tmm_2v2"] == chained_ratings["tmm_2v2"]
+    chained_ratings._get_initial_rating.assert_called_once()
+
+
 def test_initialization_with_high_deviation(chained_ratings):
     chained_ratings["ladder_1v1"] = (1000, 150)
     assert chained_ratings["ladder_1v1"] == (1000, 150)
@@ -161,6 +173,30 @@ def test_dict_update(chained_ratings):
     assert chained_ratings["global"] == (750, 100)
 
 
+def test_dict_update_caching(chained_ratings):
+    # Return a new mock on each call. By default it will return the same mock
+    chained_ratings._get_initial_rating = mock.Mock(side_effect=mock.Mock)
+
+    chained_ratings["ladder_1v1"] = (1000, 50)
+    previous_rating = chained_ratings["global"]
+    chained_ratings._get_initial_rating.assert_called_once()
+
+    chained_ratings.update({
+        "ladder_1v1": (500, 100)
+    })
+    # Global should be re-initialized after dict update
+    assert chained_ratings["global"] != previous_rating
+    assert chained_ratings._get_initial_rating.call_count == 2
+
+    chained_ratings.update({
+        "ladder_1v1": (500, 100),
+        "global": (750, 100)
+    })
+    # Global should not be re-initialized
+    assert chained_ratings["global"] == (750, 100)
+    assert chained_ratings._get_initial_rating.call_count == 2
+
+
 def test_ratings_update_same_leaderboards(chained_leaderboards):
     ratings1 = PlayerRatings(chained_leaderboards)
     ratings2 = PlayerRatings(chained_leaderboards)