Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#17634 closed Cleanup/optimization (fixed)

MultiValueDict.appendlist is ineffective

Reported by: Miloslav Pojman Owned by: nobody
Component: Core (Other) Version: 1.3
Severity: Normal Keywords:
Cc: miloslav.pojman@…, charette.s@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Current MultiValueDict.appendlist implementation uses + operator in order to merge two lists -- which is pretty ineffective.

When I replaced a list merge by a simple append call I got aprox. 20% execution time of the original implementation:

    def appendlist(self, key, value):
        super(MultiValueDict, self).setdefault(key, []).append(value)

Many calls to appendlist method are possible if the MultiValueDict is being built incrementally (for example when reading response from a remote API).

Attachments (2)

master...ticket-17634-multivalue-dict-optimisation.diff​ (592 bytes ) - added by Simon Charette 12 years ago.
master...ticket-17634-multivalue-dict-optimisation.diff.1​ (1.3 KB ) - added by Simon Charette 12 years ago.
With tests from claudep

Download all attachments as: .zip

Change History (11)

comment:1 by Miloslav Pojman, 12 years ago

Cc: miloslav.pojman@… added

comment:2 by Carl Meyer, 12 years ago

Component: Uncategorized → Core (Other)
Triage Stage: Unreviewed → Accepted

by Simon Charette, 12 years ago

Attachment: master...ticket-17634-multivalue-dict-optimisation.diff​ added

comment:3 by Simon Charette, 12 years ago

Cc: charette.s@… added
Has patch: set

Added the diff which is also reachable on ​github.

​All tests passing using test_sqlite settings with this patch applied on r17419.

comment:4 by Claude Paroz, 12 years ago

Needs tests: set

I suggest to add a test specific to appendlist in regressiontests/utils/datastructures.py (class MultiValueDictTests).

comment:5 by Claude Paroz, 12 years ago

djangobench results on MultiValueDict.appendlist with current patch:

Min: 0.000008 -> 0.000004: 2.0625x faster
Avg: 0.000008 -> 0.000005: 1.8077x faster
Significant (t=48.456398)

by Simon Charette, 12 years ago

Attachment: master...ticket-17634-multivalue-dict-optimisation.diff.1​ added

With tests from claudep

comment:6 by Simon Charette, 12 years ago

Easy pickings: set
Needs tests: unset

Added a patch with test provided by claudep.

comment:7 by Claude Paroz, 12 years ago

Triage Stage: Accepted → Ready for checkin

Thanks, looks fine now.

comment:8 by Aymeric Augustin, 12 years ago

Resolution: → fixed
Status: new → closed

In [17464]:

(The changeset message doesn't reference this ticket)

comment:7 by Aymeric Augustin, 12 years ago

In [17807]:

[1.3.X] Fixed #17634 -- Optimized the performance of MultiValueDict by using append instead of copy and by minimizing the number of dict lookups. Backport of r17464 from trunk.

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