Skip to content

Commit 6b8db7f

Browse files
Avoid per-row queries for evolution condition variables and natures (#1690)
* Load evolution variables and natures once per list of evolution details get_condition_expression and get_allowed_natures ran a query per PokemonEvolution row. Milcery -> Alcremie has 63 rows, each with a condition_expression, so its evolution chain took 65 queries. Both tables are tiny (4 evolution variables, 25 natures), so load each once, lazily, on the serializer instance. A ListSerializer reuses its child instance for every row, so this is one query per list instead of one per row, without sharing state across requests. * Cache serialized evolution variables and natures, not just the rows The cached lookups still ran the summary serializers once per evolution row. Serialize each table once alongside the query and reuse the results; serializing Alcremie's chain (63 rows) drops from ~52 ms to ~32 ms locally with identical output. Variables are kept as (symbol, data) pairs instead of a dict, since symbol isn't unique across version groups. * Remove comment --------- Co-authored-by: Alessandro Pezzè <igougi.ui@gmail.com>
1 parent 8c426c4 commit 6b8db7f

2 files changed

Lines changed: 71 additions & 9 deletions

File tree

‎pokemon_v2/serializers.py‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import itertools
66
from collections import defaultdict
7+
from functools import cached_property
78
from typing import TYPE_CHECKING, Any, ClassVar, Protocol, cast
89

910
from django.db.models import Q
@@ -3429,29 +3430,36 @@ class Meta:
34293430
"condition_expression",
34303431
)
34313432

3433+
@cached_property
3434+
def _serialized_natures(self) -> list[tuple[int, dict[str, Any]]]:
3435+
natures = Nature.objects.order_by("id")
3436+
data = NatureSummarySerializer(natures, many=True, context=self.context).data
3437+
return [(nature.pk, item) for nature, item in zip(natures, data, strict=True)]
3438+
3439+
@cached_property
3440+
def _serialized_evolution_variables(self) -> list[tuple[str, dict[str, Any]]]:
3441+
variables = EvolutionVariable.objects.order_by("pk")
3442+
data = EvolutionVariableSummarySerializer(variables, many=True, context=self.context).data
3443+
return [(variable.symbol, item) for variable, item in zip(variables, data, strict=True)]
3444+
34323445
@extend_schema_field(NatureSummarySerializer(many=True))
34333446
def get_allowed_natures(self, obj: PokemonEvolution) -> list[dict[str, Any]] | None:
34343447
if obj.nature_bitmask is None:
34353448
return None
3436-
nature_ids = [i for i in range(1, 26) if (obj.nature_bitmask & (1 << (i - 1)))]
3449+
nature_ids = {i for i in range(1, 26) if (obj.nature_bitmask & (1 << (i - 1)))}
34373450
if not nature_ids:
34383451
return None
3439-
natures = Nature.objects.filter(id__in=nature_ids).order_by("id")
3440-
return cast(
3441-
"list[dict[str, Any]]",
3442-
NatureSummarySerializer(natures, many=True, context=self.context).data,
3443-
)
3452+
return [item for nature_id, item in self._serialized_natures if nature_id in nature_ids]
34443453

34453454
@extend_schema_field(EvolutionConditionExpressionSerializer(allow_null=True))
34463455
def get_condition_expression(self, obj: PokemonEvolution) -> dict[str, Any] | None:
34473456
if not obj.condition_expression:
34483457
return None
3449-
tokens = obj.condition_expression.split()
3450-
variables = EvolutionVariable.objects.filter(symbol__in=tokens)
3458+
tokens = set(obj.condition_expression.split())
34513459
return {
34523460
"expression": obj.condition_expression,
34533461
"percentage_chance": obj.percentage_chance,
3454-
"variables": EvolutionVariableSummarySerializer(variables, many=True, context=self.context).data,
3462+
"variables": [item for symbol, item in self._serialized_evolution_variables if symbol in tokens],
34553463
}
34563464

34573465

‎pokemon_v2/tests.py‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import json
22
from datetime import datetime, timezone
33

4+
from cachalot.api import cachalot_disabled
45
from django.db import connection
56
from django.test.utils import CaptureQueriesContext
67
from rest_framework import status
@@ -5053,6 +5054,59 @@ def test_evolution_chain_api_evolution_details_order_with_many_rows_for_same_spe
50535054
expected_min_levels,
50545055
)
50555056

5057+
# condition_expression and allowed_natures must not query once per evolution row
5058+
# (e.g. Milcery -> Alcremie has 63 rows, each with a condition_expression)
5059+
def test_evolution_chain_api_condition_and_nature_lookups_do_not_scale_with_rows(self):
5060+
ec_variable = EvolutionVariable.objects.create(name="ec for evo chn", symbol="EC")
5061+
EvolutionVariable.objects.create(name="pid for evo chn", symbol="PID")
5062+
natures = [self.setup_nature_data(name=f"ntr {i} for evo chn") for i in range(3)]
5063+
allowed = [natures[0], natures[2]]
5064+
nature_bitmask = sum(1 << (nature.pk - 1) for nature in allowed)
5065+
5066+
def build_chain(row_count):
5067+
evolution_chain = self.setup_evolution_chain_data()
5068+
basic = self.setup_pokemon_species_data(
5069+
name=f"bsc for evo chn cnd {row_count}", evolution_chain=evolution_chain
5070+
)
5071+
target = self.setup_pokemon_species_data(
5072+
name=f"trgt for evo chn cnd {row_count}",
5073+
evolves_from_species=basic,
5074+
evolution_chain=evolution_chain,
5075+
)
5076+
for _ in range(row_count):
5077+
evolution = self.setup_pokemon_evolution_data(evolved_species=target)
5078+
evolution.condition_expression = "EC 100 % 0 =="
5079+
evolution.nature_bitmask = nature_bitmask
5080+
evolution.save()
5081+
return evolution_chain
5082+
5083+
small_chain = build_chain(row_count=1)
5084+
large_chain = build_chain(row_count=5)
5085+
5086+
# inside the test transaction cachalot serves repeated identical queries from
5087+
# memory, which would hide the per-row queries this test is about
5088+
with cachalot_disabled():
5089+
with CaptureQueriesContext(connection) as small_queries:
5090+
self.client.get("{}/evolution-chain/{}/".format(API_V2, small_chain.pk))
5091+
with CaptureQueriesContext(connection) as large_queries:
5092+
response = self.client.get("{}/evolution-chain/{}/".format(API_V2, large_chain.pk))
5093+
5094+
self.assertEqual(response.status_code, status.HTTP_200_OK)
5095+
self.assertEqual(len(large_queries.captured_queries), len(small_queries.captured_queries))
5096+
5097+
details = response.data["chain"]["evolves_to"][0]["evolution_details"]
5098+
self.assertEqual(len(details), 5)
5099+
for detail in details:
5100+
self.assertEqual(detail["condition_expression"]["expression"], "EC 100 % 0 ==")
5101+
self.assertEqual(
5102+
[variable["name"] for variable in detail["condition_expression"]["variables"]],
5103+
[ec_variable.name],
5104+
)
5105+
self.assertEqual(
5106+
[nature["name"] for nature in detail["allowed_natures"]],
5107+
[nature.name for nature in allowed],
5108+
)
5109+
50565110
# Encounter Tests
50575111
def test_encounter_method_api(self):
50585112
encounter_method = self.setup_encounter_method_data(name="base encntr mthd")

0 commit comments

Comments
 (0)