[Django] #35285: Optimize ForeignObject._check_unique_target

[Django]

#35285: Optimize ForeignObject._check_unique_target
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: Adam Johnson
Johnson |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Continuing my project to optimize the system checks, I found some
optimizations for `ForeignObject._check_unique_target`, which I found to
take ~6% of the total runtime for checks.

Most of this function’s runtime was spent creating the set-of-frozensets
describing all unique constraints on the target model. But this set is not
needed to determine success in the most typical cases, where a primary key
or other single unique field is targeted.

To optimize, we can check for a single unique field first. This
essentially means restoring the “fast path” replaced in #25535 /
80dac8c33e7f6f22577e4346f44e4c5ee89b648c with a broad general algorithm to
cover unique constraints.

Before optimization stats:

Profiling running checks 100 times, 20,800 calls took 180ms, or ~6% of the
total runtime.

After optimization:

29ms or ~1% of the total runtime.
--
Ticket URL: <https://code.djangoproject.com/ticket/35285>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35285: Optimize ForeignObject._check_unique_target
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted

Comment:

Accepting though I'd like confirmation that, without altering the current
logic, commenting out specific parts of the checks (via 'mutation
testing') results in failed tests. Adam, did you ensure we had sufficient
meaningful coverage before modifying the logic?
--
Ticket URL: <https://code.djangoproject.com/ticket/35285#comment:1>

[Django]

#35285: Optimize ForeignObject._check_unique_target
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

Yes, we have lots of test coverage for this check. The tests added in
80dac8c33e7f6f22577e4346f44e4c5ee89b648c are still there and there are
more in `tests/invalid_models_tests/test_relative_fields.py` like
`test_foreign_object_to_partially_unique_field` and
`test_foreign_object_to_unique_field_with_meta_constraint`.

Commenting/replacing any of the old three clauses to create
`unique_foreign_fields` causes a test failure or a check failure for test
models, preventing the tests from running. Similarly with the optimized
version.
--
Ticket URL: <https://code.djangoproject.com/ticket/35285#comment:2>

[Django]

#35285: Optimize ForeignObject._check_unique_target
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35285#comment:3>

[Django]

#35285: Optimize ForeignObject._check_unique_target
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson

Cleanup/optimization | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"e5ec11a84d6f3725220360e893050cb00403a22e" e5ec11a]:
{{{#!CommitTicketReference repository=""
revision="e5ec11a84d6f3725220360e893050cb00403a22e"
Fixed #35285 -- Optimized ForeignObject._check_unique_target().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35285#comment:4>