Avoid per-row queries for evolution condition variables and natures - #1690
santichausis wants to merge 1 commit into
Conversation
| # a ListSerializer reuses one child instance for every row, so these load once per | ||
| # list of evolution details instead of once per row | ||
| @cached_property | ||
| def _all_natures(self) -> list[Nature]: |
There was a problem hiding this comment.
would it be possible to use prefetch_related instead
There was a problem hiding this comment.
prefetch_related only works across model relations, and neither of these is one: nature_bitmask is an IntegerField and condition_expression is a CharField whose tokens are matched against EvolutionVariable.symbol. There's no FK/M2M from PokemonEvolution to Nature or EvolutionVariable to prefetch through. Getting there would mean adding M2M tables plus a migration and build/CSV changes, which felt out of scope for a perf fix. The cached_property does the equivalent of a prefetch here: one query per table per list of evolution details, filtered in Python.
If you'd prefer, I can instead preload both tables once in EvolutionChainDetailSerializer.build_chain (next to the batched evolution query from #1688) and pass them down via the serializer context, which makes it one query per request. Happy to go either way.
Also rebased onto master now that #1688 is in. With both changes, serializing all 540 evolution chains from the real data goes from 1106 to 895 queries, with byte-identical output.
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.
003f0ff to
7f96c55
Compare
2 participants
Problem
PokemonEvolutionSerializer.get_condition_expressionandget_allowed_naturesrun one query perPokemonEvolutionrow. Milcery → Alcremie has 63 evolution rows, each with acondition_expression, so/evolution-chain/452/takes 65 queries (63 of them topokemon_v2_evolutionvariable).Fix
Both tables are tiny (4 evolution variables, 25 natures). Each is loaded once, lazily, through a
cached_propertyon the serializer instance and filtered in Python, keeping the previous ordering.ListSerializerreuses a single child instance for every row, so this is one query per list of evolution details instead of one per row, and no state is shared across requests.Verification
condition_expressionandnature_bitmask), asserts the query count is the same, and checks the resolved variables and natures. This is the first API test coverage for these two fields. It usescachalot_disabled(), because inside the test transaction cachalot serves identical repeated queries from memory, which hides per-row queries. On master this test fails with 13 vs 5 queries.manage.py test pokemon_v2(64 tests), ruff, and ty (no new diagnostics) pass.