#9546 closed bug (fixed)

Opened September 10, 2013 07:19AM UTC

Closed July 24, 2014 04:08PM UTC

Last modified July 24, 2014 04:12PM UTC

cleanData() proxy is slow

Reported by:	frederik.elvhage	Owned by:	frederik.elvhage
Priority:	minor	Milestone:	1.11.1
Component:	ui.widget	Version:	1.10.1
Keywords:		Cc:
Blocked by:		Blocking:

Description

The (jQuery-UI) cleanData-Proxy i find in my jquery-ui-1.10.1.custom.js slows down ajax-calls (especially) in internet explorer 8.

I have found that removing this proxy will reduce ajax-processing time by more than 60% in IE8 and on large ajax-calls (~3.3s -> 1.1s).

It is obviously a counter-measure for a jquery-bug that has been fixed in 1.6.3.

$.cleanData = function( elems ) {
	for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) {
		try {
			$( elem ).triggerHandler( "remove" );
		// http://bugs.jquery.com/ticket/8235
		} catch( e ) {}
	}
	_cleanData( elems );
};

I suggest removing this proxy and change legacy-support to 1.6.3+ (instead of 1.6+).

Attachments (0)

Change History (12)

Changed September 10, 2013 01:35PM UTC by tj.vantoll comment:1

scott.gonzalez: Have we discussed what versions of jQuery core we're going to support in 1.11? This seems like a decent reason to drop 1.6.

Changed September 10, 2013 02:05PM UTC by scottgonzalez comment:2

component:	ui.core → ui.widget
owner:	→ frederik.elvhage
status:	new → pending

This proxy has nothing to do with working around any bugs. This is the implementation of the remove event for auto-destruction of widgets. The comment with the bug URL is pointing to the reason that they try/catch exists. The same try/catch was added in jQuery core 1.6.3. I think you're confused about what's going on here.

It sounds like you're probably doing something specific beyond just making an ajax request. Can you please provide a reduced test case showing the performance problem you're having?

Changed September 10, 2013 10:09PM UTC by frederik.elvhage comment:3

status:	pending → new

Replying to [comment:2 scott.gonzalez]:

This proxy has nothing to do with working around any bugs. This is the implementation of the remove event for auto-destruction of widgets. The comment with the bug URL is pointing to the reason that they try/catch exists. The same try/catch was added in jQuery core 1.6.3. I think you're confused about what's going on here. It sounds like you're probably doing something specific beyond just making an ajax request. Can you please provide a reduced test case showing the performance problem you're having?

Ok, i (now) understand that the cleanData-Method has been proxied for other reasons than the bug mentioned.

Looking at it now, i'm not sure what may be the reason for it. It can se a significant difference even when the dom is small (percentagevise).

How i got here? I used dynatrace Ajax-edition to analyze the requests and the cleanData-Method showed up very high in the hotspots view. I looked into the jQuery-UI code and found the proxy. Removed it and found client-processing was 60% faster.

I will check the code tomorrow to see if we have some expensive remove-handlers that make this problem specific to our code. It has to be the additional iteration itself or the handlers.

What we are doing? Wicket6-Applications (jQuery) with ajax. In some very large forms the dom can get huge (10 Tabs). But as said, i can see a great difference in small dom-trees as well.

Thank you for your time.

Changed September 11, 2013 05:16AM UTC by scottgonzalez comment:4

status:	new → pending

We'll wait for the reduced test case to investigate. You may find something just by reducing your code.

Changed September 11, 2013 02:01PM UTC by frederik.elvhage comment:5

_comment0:	I made a fiddle: \ \ [http://jsfiddle.net/felvhage/Sj769/ jsfiddle/slow] \ \ It replaces some arbitrary content with and without cleanData and leaves me with the following results (see fiddle)[[BR]] \ \ replacing content with jQuery + jQuery-UI: ~30ms [[BR]] \ \ replacing content with jQuery + jQuery-UI but without cleanData: ~3ms[[BR]] \ \ replacing content with jQuery only: ~9ms[[BR]] \ \ I understand that jQuery-UI must cause some overhead, but this seems a bit too much to me. \ → 1378912240388716
status:	pending → new

_comment0:

I made a fiddle: \ \ [http://jsfiddle.net/felvhage/Sj769/ jsfiddle/slow] \ \ It replaces some arbitrary content with and without cleanData and leaves me with the following results (see fiddle)[[BR]] \ \ replacing content with jQuery + jQuery-UI: ~30ms [[BR]] \ \ replacing content with jQuery + jQuery-UI but without cleanData: ~3ms[[BR]] \ \ replacing content with jQuery only: ~9ms[[BR]] \ \ I understand that jQuery-UI must cause some overhead, but this seems a bit too much to me. \ → 1378912240388716

status:

pending → new

I made a fiddle:

jsfiddle/slow

It replaces some arbitrary content with and without cleanData and leaves me with the following results (see fiddle)

replacing content with jQuery + jQuery-UI: ~30ms

replacing content with jQuery + jQuery-UI but without cleanData: ~3ms

replacing content with jQuery only: ~9ms

I understand that jQuery-UI must cause some overhead, but this seems a bit too much to me.

It makes a difference in all browsers. IE8 is just where we see performance-problems first.

Changed September 11, 2013 03:12PM UTC by tj.vantoll comment:6

Thanks for the test case. I'm seeing a difference of...

Chrome latest: ~5ms (no jQuery UI) vs. ~30ms (with jQuery UI)

IE8: ~200ms (no jQuery UI) vs. ~600ms (with jQuery UI)

While that's non-trivial, there are 2500+ DOM nodes being removed. Off the top of my head I can't think of a good way to speed this up. We could try to be smarter about when we need to trigger the

remove

, but of course there's a cost to doing the detection.

Changed September 11, 2013 03:15PM UTC by scottgonzalez comment:7

resolution:	→ patcheswelcome
status:	new → closed

We've looked at other ways to implement the remove event, but we haven't found anything better. Feel free to send a pull request if you have an idea.

Changed September 12, 2013 07:37AM UTC by frederik.elvhage comment:8

_comment0:	Update: there was a misunderstanding in the original bug-description. [[BR]] \ \ The (jQuery-UI) cleanData-Proxy is used to implement the (still needed) jQuery-UI remove-Method. [[BR]] \ The actual problem is, that triggering remove on every element in the dom is expensive. \ \ This is browser independent but will hit you harder in IE. \ \ Replying to [ticket:9546 frederik.elvhage]: \ > The (jQuery-UI) cleanData-Proxy i find in my jquery-ui-1.10.1.custom.js slows down ajax-calls (especially) in internet explorer 8. \ > \ > I have found that removing this proxy will reduce ajax-processing time by more than 60% in IE8 and on large ajax-calls (~3.3s -> 1.1s). \ > \ > It is obviously a counter-measure for a jquery-bug that has been fixed in 1.6.3. \ > {{{ \ > $.cleanData = function( elems ) { \ > for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) { \ > try { \ > $( elem ).triggerHandler( "remove" ); \ > // http://bugs.jquery.com/ticket/8235 \ > } catch( e ) {} \ > } \ > _cleanData( elems ); \ > }; \ > }}} \ > \ > I suggest removing this proxy and change legacy-support to 1.6.3+ (instead of 1.6+). \ → 1378972724602541

_comment0:

Update: there was a misunderstanding in the original bug-description. [[BR]] \ \ The (jQuery-UI) cleanData-Proxy is used to implement the (still needed) jQuery-UI remove-Method. [[BR]] \ The actual problem is, that triggering remove on every element in the dom is expensive. \ \ This is browser independent but will hit you harder in IE. \ \ Replying to [ticket:9546 frederik.elvhage]: \ > The (jQuery-UI) cleanData-Proxy i find in my jquery-ui-1.10.1.custom.js slows down ajax-calls (especially) in internet explorer 8. \ > \ > I have found that removing this proxy will reduce ajax-processing time by more than 60% in IE8 and on large ajax-calls (~3.3s -> 1.1s). \ > \ > It is obviously a counter-measure for a jquery-bug that has been fixed in 1.6.3. \ > {{{ \ > $.cleanData = function( elems ) { \ > for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) { \ > try { \ > $( elem ).triggerHandler( "remove" ); \ > // http://bugs.jquery.com/ticket/8235 \ > } catch( e ) {} \ > } \ > _cleanData( elems ); \ > }; \ > }}} \ > \ > I suggest removing this proxy and change legacy-support to 1.6.3+ (instead of 1.6+). \ → 1378972724602541

I believe that cleaning up dom on removal is a general concept (to avoid memory leaks aso) so there should be a possiblity to register with the framework for cleanup/remove. So jQuery should offer a callback or something.

In this particular case such a callback would save the extra iteration through all elements.

Doing some further testing i found that the iteration is cheap. The triggerHandler('remove') is whats expensive regardless of attached handlers. The solution would be to trigger the remove-handler soley on elements with widgets attached.

I.e. the widget could set a flag on initialization and check it in the cleanData-Method.

I have made another fiddle that allows testing of different cleanData-Implementations here:

jsfiddle/cleanData

My first naive implementation will test for the existance of events before it calls the remove-handler.

This already saves at least 90% of the overhead. I will look further into this in time an provide a pull request.

Changed July 24, 2014 01:54PM UTC by scottgonzalez comment:9

resolution:	patcheswelcome
status:	closed → reopened

PR: https://github.com/jquery/jquery-ui/pull/1291

Changed July 24, 2014 04:08PM UTC by Frederik Elvhage comment:10

resolution:	→ fixed
status:	reopened → closed

Widget: Improve performance of cleanData method

Fixes #9546

Closes gh-1291

Changeset: f7429edfe96d322cdec850f7207efba8125767a6

Changed July 24, 2014 04:09PM UTC by scottgonzalez comment:11

milestone:	none → 1.11.1

Changed July 24, 2014 04:12PM UTC by scottgonzalez comment:12

summary:	cleanData-Proxy slows down ajax in IE8 → cleanData() proxy is slow

Search and Top Navigation