[Django] #32901: BaseForm.__getitem__() does unneeded work in the happy path

31 views
Skip to first unread message

Django

unread,
Jul 4, 2021, 10:21:01 AM7/4/21
to django-...@googlegroups.com
#32901: BaseForm.__getitem__() does unneeded work in the happy path
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: Chris Jerdonek
Jerdonek |
Type: | Status: assigned
Cleanup/optimization |
Component: Forms | Version: dev
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 |
-------------------------------------+-------------------------------------
I noticed that in the "happy path," `BaseForm.__getitem__()` does unneeded
work:
https://github.com/django/django/blob/fa35c8bdbc6aca65d94d6280fa463d5bc7baa5c0/django/forms/forms.py#L150-L164

It can just return `self._bound_fields_cache[name]` at the beginning and
handle `KeyError`, instead of accessing `self.fields` followed by checking
for the presence of `name` in `self._bound_fields_cache` before doing so
each time.

--
Ticket URL: <https://code.djangoproject.com/ticket/32901>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 5, 2021, 12:05:42 AM7/5/21
to django-...@googlegroups.com
#32901: BaseForm.__getitem__() does unneeded work in the happy path
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Unreviewed => Accepted


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

Django

unread,
Jul 5, 2021, 6:37:28 AM7/5/21
to django-...@googlegroups.com
#32901: BaseForm.__getitem__() does unneeded work in the happy path
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Forms | Version: dev

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 Chris Jerdonek):

* has_patch: 0 => 1


Comment:

PR: https://github.com/django/django/pull/14596

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

Django

unread,
Jul 5, 2021, 7:34:44 AM7/5/21
to django-...@googlegroups.com
#32901: BaseForm.__getitem__() does unneeded work in the happy path
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Forms | Version: dev
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/32901#comment:3>

Django

unread,
Jul 5, 2021, 9:21:33 AM7/5/21
to django-...@googlegroups.com
#32901: BaseForm.__getitem__() does unneeded work in the happy path
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
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 GitHub <noreply@…>):

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


Comment:

In [changeset:"edde2a069929c93e37835dc3f7c9a229040058e2" edde2a0]:
{{{
#!CommitTicketReference repository=""
revision="edde2a069929c93e37835dc3f7c9a229040058e2"
Fixed #32901 -- Optimized BaseForm.__getitem__().
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32901#comment:4>

Django

unread,
Nov 18, 2022, 2:36:18 AM11/18/22
to django-...@googlegroups.com
#32901: BaseForm.__getitem__() does unneeded work in the happy path
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Forms | Version: dev

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
-------------------------------------+-------------------------------------

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

In [changeset:"51faf4bd172cd4cb219a9793facbfa00246c9f3c" 51faf4bd]:
{{{
#!CommitTicketReference repository=""
revision="51faf4bd172cd4cb219a9793facbfa00246c9f3c"
Fixed #34148 -- Reverted "Fixed #32901 -- Optimized
BaseForm.__getitem__()."

This reverts commit edde2a069929c93e37835dc3f7c9a229040058e2.

Thanks Jan Pieter Waagmeester for the report.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32901#comment:5>

Reply all
Reply to author
Forward
0 new messages