[Django] #35246: Make Field.unique a plain attribute

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
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: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Another candidate for caching, like #35230, #35232 and #35241, following
the same system check profiling.

`Field.unique` is a simple property that computes whether a field is
unique from two inputs:

{{{
@property
def unique(self):
return self._unique or self.primary_key
}}}

The result is immutable because the two input attributes shouldn’t change.

I found this method was called 3543 times during system checks, taking
~0.7% (~0.3ms) of the total runtime on a Python 3.12 project with 118
models. After moving it to a plain attribute, this cost is eliminated.
(cProfile’s overhead biases the cost of function calls upwards, so the
actual saving may be smaller, but probably not too much smaller.)

`unique` is accessed in many other code paths so this change will help
those paths too.
--
Ticket URL: <https://code.djangoproject.com/ticket/35246>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

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

Changes (by Adam Johnson):

* has_patch: 0 => 1


Old description:

> Another candidate for caching, like #35230, #35232 and #35241, following
> the same system check profiling.
>
> `Field.unique` is a simple property that computes whether a field is
> unique from two inputs:
>
> {{{
> @property
> def unique(self):
> return self._unique or self.primary_key
> }}}
>
> The result is immutable because the two input attributes shouldn’t
> change.
>
> I found this method was called 3543 times during system checks, taking
> ~0.7% (~0.3ms) of the total runtime on a Python 3.12 project with 118
> models. After moving it to a plain attribute, this cost is eliminated.
> (cProfile’s overhead biases the cost of function calls upwards, so the
> actual saving may be smaller, but probably not too much smaller.)
>
> `unique` is accessed in many other code paths so this change will help
> those paths too.

New description:

Another candidate for caching, like #35230, #35232 and #35241, following
the same system check profiling.

`Field.unique` is a simple property that computes whether a field is
unique from two inputs:

{{{
@property
def unique(self):
return self._unique or self.primary_key
}}}

The result is immutable because the two input attributes shouldn’t change.

I found this method was called 3543 times during system checks, taking
~0.7% (~0.3ms) of the total runtime on a Python 3.12 project with 118
models. After moving it to a plain attribute, this cost is eliminated.
(cProfile’s overhead biases the cost of function calls upwards, so the

actual saving may be smaller, but it still seems worth the minimal
change.)

`unique` is accessed in many other code paths so this change will help
those paths too.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:1>

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Adam Johnson:

Old description:

> Another candidate for caching, like #35230, #35232 and #35241, following
> the same system check profiling.
>
> `Field.unique` is a simple property that computes whether a field is
> unique from two inputs:
>
> {{{
> @property
> def unique(self):
> return self._unique or self.primary_key
> }}}
>
> The result is immutable because the two input attributes shouldn’t
> change.
>
> I found this method was called 3543 times during system checks, taking
> ~0.7% (~0.3ms) of the total runtime on a Python 3.12 project with 118
> models. After moving it to a plain attribute, this cost is eliminated.
> (cProfile’s overhead biases the cost of function calls upwards, so the

> actual saving may be smaller, but it still seems worth the minimal
> change.)
>
> `unique` is accessed in many other code paths so this change will help
> those paths too.

New description:

Another candidate for caching, like #35230, #35232 and #35241, following
the same system check profiling.

`Field.unique` is a simple property that computes whether a field is
unique from two inputs:

{{{
@property
def unique(self):
return self._unique or self.primary_key
}}}

The result is immutable because the two input attributes shouldn’t change.

I found this method was called 3543 times during system checks, taking
~0.7% (~0.3ms) of the total runtime on a Python 3.12 project with 118
models. After moving it to a plain attribute, this cost is eliminated.
(cProfile’s overhead biases the cost of function calls upwards, so the
actual saving may be smaller, but it still seems worth the minimal
change.)

`unique` is accessed in many other code paths so this change will help
those paths too.

--
--

Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:2>

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Adam, I greatly appreciate the time you are spending investigating these
but could we please [https://github.com/django/django-asv create a
benchmark] so we can measure the impact of them sooner than later.

I understand that we can't create a benchmark for every performance
improvement we make like we do when fixing bug with regression tests but I
think the volume of these warrant creating a prior baseline so we can make
sure all the work you are investing today doesn't disappear over time as
we add more checks and features to Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:3>

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
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:

I agree with Simon on both items, that this optimization can be helpful
and that we could use some metrics to do some follow up in the medium/long
term.

I just checked the branch and wonder if converting the property into a
cached one, instead of deleting it in favor of an attribute, might be a
better idea. I'm thinking of potential usages out there in the wild
overriding the property for <reasons> and the proposed change would break
those scenarios.
--
Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:4>

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
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):

See https://github.com/django/django-asv/pull/80 for a benchmark, plus the
other general maintenance PRs I opened on django-asv :)
--
Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:5>

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
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):

Sure, we can use cached_property; it has the same performance after the
first call.
--
Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:6>

[Django]

#35246: Make Field.unique a plain attribute
-------------------------------------+-------------------------------------
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: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GitHub <noreply@…>):

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

Comment:

In [changeset:"e65deb7d14a57ba788b978d50bd8198e659faa91" e65deb7]:
{{{#!CommitTicketReference repository=""
revision="e65deb7d14a57ba788b978d50bd8198e659faa91"
Fixed #35246 -- Made Field.unique a cached property.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35246#comment:7>