Opened 14 years ago

Closed 12 years ago

Last modified 12 years ago

#4759 closed feature (fixed)

a major optimization is possible in sortable()

Reported by: mesoconcepts Owned by:
Priority: major Milestone: 1.8.18
Component: ui.sortable Version: 1.7.2
Keywords: Cc:
Blocked by: Blocking:


found this while writing a patch with WP bug 10021:

Basically, the key optimization was that:

  connectWith: 'bar'

is tremendously slower than:

$('foo').sortable().sortable('option', 'connectWith', 'bar');

and by this, I'm really meaning slower by inordinate amounts of time. Like... by a factor of 5 in my case, with 15 placeholders.

I haven't looked into your code's specific, but I would assume you're running an O(n!) algorithm in there instead of an O(n).

The connectWidth option, when applied to an array of objects, should be ignored, and applied at the very end, once all of the array elements are instantiated. Else, it recursively jQueries the document for the needed elements, and does the same job n! times on the first element.

I'm pretty certain you've similar optimizations to do over in draggable and droppable, and possibly other areas.

Change History (9)

comment:1 Changed 14 years ago by mesoconcepts

Some further information on this one. I've spotted a somewhat related issue... Possibly the same, too. In Opera 9.6, the init code goes:

		var o = this.options;
		this.containerCache = {};

		//Get the items

		//Let's determine if the items are floating
		this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false;

		//Let's determine the parent's offset
		this.offset = this.element.offset();

		//Initialize mouse events for interaction

each of:

  • this.refresh();
  • this.floating = ...
  • this.offset = this.element.offset();
  • this._mouseInit();

Makes use of the rendering engine in a way or another, and ends up causing several seconds of lag on the huge page I'm doing tests on.

See also this and its follow-up comment:

And, along the same lines, I've noted that adding an arbitrary ID to the page using js before the ready event is fired, or at the beginning of the ready event, will cause massive lags in Opera. The bug has been reported upstream, but it's likely something that should be worked around in UI until it's fixed.

comment:2 Changed 13 years ago by Scott González

Priority: criticalmajor
Type: bugfeature

comment:3 Changed 12 years ago by jdmarshall

What seems to be happening is that if connectWith is turned on, each container tries to cache the position and dimensions for for all the items in all of the connected sortables, instead of just caching its own.

Given that connectWith is probably most often used when connecting containers to each other (ie containerA and containerB are mutually sortable), it would be better if each container only cached its own stuff. There is also a bit of cache invalidation going on here that I haven't fully comprehended, but that may be a source of the O(n!) behavior as well.

The problem may also be due to the fact that sortable() is called on the elements directly. Each container is not aware that it has peers that are being configured at the same time. Maybe a fixed version of this would be to implement jQuery.sortable(containers, options), where it can then be smart enough to avoid recalculating cache data.

comment:4 Changed 12 years ago by jdmarshall

Note that #5886 was closed as a duplicate of this issue, but has some benchmarks.

comment:5 Changed 12 years ago by jdmarshall

I am seeing about a 15-20% improvement to sortable() with jQuery 1.7 (vs 1.5), however this change appears to be due entirely to optimization in jQuery internals. A leaf method in the CSS code runs faster, giving a linear speedup.

sortable() still represents more than 60% of the page load time in our app, so it'd be good to get this fixed sooner rather than later.

Last edited 12 years ago by jdmarshall (previous) (diff)

comment:6 Changed 12 years ago by SpoonNZ

I've made a proposed fix at

This fixes the issue, though of course may not be the best way!

Basically all it does is stop the if(connectWith) conditional within the _refreshItems() function from running until after the sortable is completely initiated.

Last edited 12 years ago by SpoonNZ (previous) (diff)

comment:7 Changed 12 years ago by SpoonNZ

Resolution: fixed
Status: newclosed

Sortable: Added a variable to track if initialization is complete. Fixes #4759 - a major optimization is possible in sortable().

Changeset: ba6916f22ac3fac993975abc0f86d6cb0bf9c08d

comment:8 Changed 12 years ago by SpoonNZ

Sortable: Added a variable to track if initialization is complete. Fixes #4759 - a major optimization is possible in sortable(). (cherry picked from commit ba6916f22ac3fac993975abc0f86d6cb0bf9c08d)

Changeset: b00faa95d0d372f345e24f9abe9d16a2b67ca258

comment:9 Changed 12 years ago by Scott González

Milestone: 1.next1.8.18
Note: See TracTickets for help on using tickets.