#34580 closed Cleanup/optimization (fixed)
Performance regession in SQLCompiler
Reported by: | David Smith | Owned by: | David Smith |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.2 |
Severity: | Release blocker | Keywords: | |
Cc: | Simon Charette | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Looking at the Django's ASV benchmarks the query count and aggregate benchmarks show performance regressions.
Rough orders of magnitude are
Query Count 1.257ms -> 1.849ms 47% Increase
Aggregate 0.882ms -> 1.238ms 40% Increase
I managed to bisect the regression to this commit https://github.com/django/django/commit/278881e3 |
278881e37619278789942513916acafaa88d26f3
Change History (7)
comment:1 by , 18 months ago
Cc: | added |
---|---|
Component: | Uncategorized → Database layer (models, ORM) |
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 18 months ago
I attempted to see if the benchmark workflow could detect the changes from the propose patch in PR . Unfortunately it could not.
However, running locally I can see a performance improvement in the region of 20-30% from the proposed change. I was able to repeat this, and see a similar improvement a second time.
before after ratio [ca5d3c99] [2aa6996a] <form_docs> <SQLCompiler> - 1.83±0.01ms 1.41±0.01ms 0.77 query_benchmarks.query_exists.benchmark.QueryExists.time_query_exists - 874±2μs 621±8μs 0.71 query_benchmarks.query_aggregate.benchmark.QueryAggr.time_aggregate - 1.35±0.02ms 905±4μs 0.67 query_benchmarks.query_count.benchmark.QueryCount.time_query_count
comment:3 by , 18 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 18 months ago
Owner: | changed from | to
---|---|
Severity: | Normal → Release blocker |
Status: | new → assigned |
Triage Stage: | Accepted → Ready for checkin |
comment:5 by , 18 months ago
Has patch: | set |
---|
Thanks for the report, can you point at the exact test definition that regressed otherwise it's quite hard to confirm what actually got slower
Is this
query_count and aggregate?
If that's the case then I'm surprised this particular commit caused a regression in these
Count
tests as none of them make use or ordering. If that's truly the culprit then it's likely only a matter of not generatingselected_exprs
if there is noordering
in the first place.django/db/models/sql/compiler.py
Is there any way to confirm that the above patch addresses the regression? Can we run performance tests against the baseline from a particular PR?