Opened 2 months ago

Closed 2 months ago

#35285 closed Cleanup/optimization (fixed)

Optimize ForeignObject._check_unique_target

Reported by: Adam Johnson Owned by: Adam Johnson
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: 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

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.

Change History (4)

comment:1 by Natalia Bidart, 2 months ago

Triage Stage: UnreviewedAccepted

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?

comment:2 by Adam Johnson, 2 months ago

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.

comment:3 by Mariusz Felisiak, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In e5ec11a:

Fixed #35285 -- Optimized ForeignObject._check_unique_target().

Note: See TracTickets for help on using tickets.
Back to Top