Code

Opened 7 years ago

Closed 7 years ago

#3099 closed enhancement (duplicate)

New faster SelectBox.js

Reported by: Graham King <graham@…> Owned by: adrian
Component: contrib.admin Version:
Severity: normal Keywords:
Cc: graham@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

If you have thousands of entries in a multiple select box in the Django admin interface, the Javascript in SelectBox.js which makes the nice gui widget, runs too slowly, and the browser will complain.

Here is a new SelectBox.js that uses maps/objects where it used to use lists, changing the complexity of SelectBox.move from quadratic to

linear (and I thought big O notation was just for interview questions!). For example on a list of 4000 elements it now iterates 4000 times instead of up to 16 Million (4000*4000).

This also always displays the entries sorted alphabetically. This was, I _think_, the intention of the original SelectBox, but it wasn't happening in practice. Aside from this behaviour is identical to previous SelectBox.

Attachments (2)

SelectBox.js (3.6 KB) - added by Graham King <graham@…> 7 years ago.
Replacement for django/contrib/admin/media/js/SelectBox
3099.diff (5.2 KB) - added by Gary Wilson <gary.wilson@…> 7 years ago.
a patch instead of the whole file. also fixed indention and spacing.

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Graham King <graham@…>

Replacement for django/contrib/admin/media/js/SelectBox

comment:1 Changed 7 years ago by Gary Wilson <gary.wilson@…>

The attached patch also seems to fix #348.

comment:2 Changed 7 years ago by Gary Wilson <gary.wilson@…>

see also #3202.

comment:3 Changed 7 years ago by Graham King <graham@…>

  • Cc graham@… added

comment:4 Changed 7 years ago by Graham King <graham@…>

  • Has patch set

comment:5 follow-up: Changed 7 years ago by SmileyChris

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Can someone compare #3202 and this and figure out the better one?

comment:6 in reply to: ↑ 5 Changed 7 years ago by gkelly@…

Replying to SmileyChris:

Can someone compare #3202 and this and figure out the better one?

Performance wise, this patch actually did worse in my testing than the original (I'm attaching a screenshot of the Firebug profiler as evidence). Using an Object rather than Array doesn't seem to speed up anything. Also note how much time the sort method takes compared to the original and #3202. It is much more time here. If this patch is intended to fix a sorting bug, then another patch should be submitted after taking into account the changes in patch #3202.

comment:7 Changed 7 years ago by gkelly@…

I'm getting an error when trying to upload an attachment. I posted to Django-developers and hopefully it will be fixed soon so I can upload the screenshot of the javascript profiler.

comment:8 Changed 7 years ago by Graham King <graham@…>

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

I've done some Firebug profiling as well, and I agree. This version (which I wrote) is actually about a second slower than the original in Firefox. I tested #3202 and it is a good bit faster than the original, so for Firefox my vote goes with that. Apologies for wasting time.

Neither this nor #3202 seem to make any difference to IE though. I'm going to close this and add my comments to #3202 about what we might be able to do about IE.

Closing as duplicate of #3202.

Changed 7 years ago by Gary Wilson <gary.wilson@…>

a patch instead of the whole file. also fixed indention and spacing.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.