Opened 4 years ago

Closed 22 months ago

Last modified 22 months ago

#15625 closed Cleanup/optimization (fixed)

MultiValueDictKeyError exception has large performance penalty

Reported by: margieroginski Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am seeing a large potential performance penalty due to a string argument representing the entire POST dict getting created when a key is not found in the POST dict.

In datastructures.py, in class MultiValueDict, __getitem__() has the following code:

        try:
            list_ = super(!MultiValueDict, self).__getitem__(key)
        except !KeyError:
            raise !MultiValueDictKeyError("Key %r not found in %r" % (key, self))

During form validation, this exception is raised for each field/key not found in the POST dict. If the POST dict is very large, the %r associated with self is very large and can have a very significant time penalty. In a case where I had 300 of these exceptions, the time penalty of creating this string was almost 2 seconds.

It is not that unusual to have POST keys missing that are checked in this way. Here are a few of the common cases I've seen:

  1. checkboxes that are unchecked
  2. url arguments that could be POSTED but which are omitted
  3. fields that are dynamic - ie, they may not be there unless some client code creates them.

Do we need to provide a string argument at all in this case, given the potential time penalty and the frequency of getting values from the POST dict?

Change History (7)

comment:1 Changed 4 years ago by adrian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This is a fair point. I would be in favor of changing the exception to match that of the built-in dictionary type: KeyError: 'key'.

comment:2 Changed 4 years ago by lukeplant

  • Type set to Cleanup/optimization

comment:3 Changed 4 years ago by lukeplant

  • Severity set to Normal

comment:4 Changed 4 years ago by thejaswi_puthraya

  • Component changed from Uncategorized to HTTP handling
  • Easy pickings unset
  • UI/UX unset

comment:5 Changed 22 months ago by timo

  • Has patch set
  • Version changed from 1.2 to master

comment:6 Changed 22 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 893198509e3b821a56a56e4326929e0613aad983:

Fixed #15625 -- Made message in MultiValueDictKeyError less verbose.

Thanks margieroginski for the suggestion.

comment:7 Changed 22 months ago by Tim Graham <timograham@…>

In 275497c5709792b47c71538a7e58bb40e8d8d3a9:

[1.6.x] Fixed #15625 -- Made message in MultiValueDictKeyError less verbose.

Thanks margieroginski for the suggestion.

Backport of 893198509e from master

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