Skip to content

Improve placement priority performance#1209

Open
davidfischer wants to merge 1 commit intomainfrom
davidfischer/improve-placement-priority-perf
Open

Improve placement priority performance#1209
davidfischer wants to merge 1 commit intomainfrom
davidfischer/improve-placement-priority-perf

Conversation

@davidfischer
Copy link
Copy Markdown
Collaborator

Looking at the performance degradation in NR, the problem didn't seem to be a "query" problem in so far as NR didn't show elevated database times. Looking at the code, this led me to believe the degradation (~20ms avg) from the previous change was due to python deserializing too many objects from the database.

After discussion with the ol' AI (Gemini), this change should avoid deserializing a pretty large number of objects and be similar to what we had before. Previously, we were requesting every ad and its ad types for each ad from each candidate flight. Assuming 3 ad types per ad and ~30 flights and ~4 ads per flight, we're looking at ~350 more objects being deserialized from the DB. The query itself didn't appear more expensive, but turning that data into Django model objects was. That's at least the working theory.

I have verified that this change looks good from a query perspective and I reviewed every query in the critical path.

References

Looking at the performance degradation in NR, the problem didn't seem to
be a "query" problem in so far as NR didn't show elevated database
times. Looking at the code, this led me to believe the degradation
(~20ms avg) from the previous change was due to python deserializing too
many objects from the database.

After discussion with the ol' AI (Gemini), this change should avoid
deserializing a pretty large number of objects and be similar to what we
had before. Previously, we were requesting every ad and its ad types for
each ad from each candidate flight. Assuming 3 ad types per ad and ~30
flights and ~4 ads per flight, we're looking at ~350 more objects being
deserialized from the DB. The query itself didn't appear more expensive,
but turning that data into Django model objects was. That's at least the
working theory.

I have verified that this change looks good from a query perspective and
I reviewed every query in the critical path.
@davidfischer davidfischer requested a review from a team as a code owner April 18, 2026 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant