Unresponsive script errors on widgets screen

[Denis-de-Bernardy]

I'm getting unresponsive script errors in Camino on the widgets screen, when multitudes of widgets and widget fields are present.

I've nailed this down to an optimization that is possible in the various jQuery functions. Specifically, the likes of find(".class") can be changed to find("tag.class") for the same functionality *and* a faster selector.

Attached patch does enough for the error message to go away while keeping the functionality intact.

[Denis-de-Bernardy]

per IRC discussion

[Denis-de-Bernardy]

Soo.. I added a logger in there (patch to make it appear is attached). And the breakdown for my screen is the following:

0ms -- adminMenu.init()
19ms -- $('#adminmenu div.wp-menu-toggle').each
5ms -- this.favorites();
4ms -- $('a.separator').click
1ms -- $('body').hasClass('folded')
8ms -- this.restoreMenuState();
1ms -- end adminMenu.init()
0ms -- columns.init()
4ms -- end columns.init()
0ms -- commons.ready()
4ms -- $('.fade').animate
7ms -- $('div.wrap h2 ~ div.updated, div.wrap h2 ~ div.error').addClass('below-h2');
12ms -- $('div.updated, div.error').not('.below-h2').insertAfter('div.wrap h2:first');
5ms -- $('#doaction, #doaction2').click
1ms -- $('#show-settings-link').click
1ms -- $('#contextual-help-link').click
6ms -- $('#contextual-help-link-wrap, #screen-options-link-wrap').show();
743ms -- $( 'table:visible tbody .check-column :checkbox' ).click
227ms -- $( 'thead :checkbox, tfoot :checkbox' ).click
3ms -- $('#default-password-nag-no').click
1ms -- end commons.ready()
1ms -- turbo.ready()
7ms -- end turbo.ready()
1ms -- widgets.init()
1ms -- $('body').hasClass('widgets_access')
7ms -- $('#widgets-right div.sidebar-name').click
6ms -- $('#widgets-left div.sidebar-name').click
45ms -- $('#widgets-right .widget, #wp_inactive_widgets .widget').each
35ms -- this.addEvents();
4ms -- $('.widget-error').parents('.widget').find('a.widget-action').click();
8ms -- $('#available-widgets').droppable
31ms -- $('#widget-list .widget').draggable
1092ms -- $('.widgets-sortables').sortable
10ms -- wpWidgets.resize();
367ms -- wpWidgets.fixLabels();
1ms -- end widgets.init()

[Denis-de-Bernardy]

the logger patch is against today's 2.8 branch

[Denis-de-Bernardy]

oh, the latter is for a widgets screen with lots of checkboxes, btw.

[Denis-de-Bernardy]

Soo... making good progress...

0ms -- adminMenu.init()
12ms -- $('#adminmenu div.wp-menu-toggle').each
5ms -- this.favorites();
3ms -- $('a.separator').click
1ms -- $('body').hasClass('folded')
5ms -- this.restoreMenuState();
0ms -- end adminMenu.init()
1ms -- columns.init()
3ms -- end columns.init()
0ms -- commons.ready()
3ms -- $('.fade').animate
5ms -- $('div.wrap h2 ~ div.updated, div.wrap h2 ~ div.error').addClass('below-h2');
9ms -- $('div.updated, div.error').not('.below-h2').insertAfter('div.wrap h2:first'); --- potential optimization left
3ms -- $('#doaction, #doaction2').click
1ms -- $('#show-settings-link').click
0ms -- $('#contextual-help-link').click
7ms -- $('#contextual-help-link-wrap, #screen-options-link-wrap').show();
8ms -- $( 'table:visible tbody .check-column :checkbox' ).click
4ms -- $( 'thead :checkbox, tfoot :checkbox' ).click
2ms -- $('#default-password-nag-no').click
1ms -- end commons.ready()
0ms -- turbo.ready()
6ms -- end turbo.ready()
1ms -- widgets.init()
1ms -- $('body').hasClass('widgets_access')
8ms -- $('#widgets-right div.sidebar-name').click
5ms -- $('#widgets-left div.sidebar-name').click
106ms -- $('#widgets-right .widget, #wp_inactive_widgets .widget').each
46ms -- this.addEvents();
4ms -- $('.widget-error').parents('.widget').find('a.widget-action').click();
4ms -- $('#available-widgets').droppable
29ms -- $('#widget-list .widget').draggable
1071ms -- $('.widgets-sortables').sortable
9ms -- wpWidgets.resize();
529ms -- wpWidgets.fixLabels();
1ms -- end widgets.init()

[aaroncampbell]

Just adding a quick note here for others like me that are a little out of the loop on jQuery's inner workings.

My numbers were pretty different from the ones noted above because I had far fewer widgets. However, $( 'table:visible tbody .check-column :checkbox' ).click was taking 40-45 ms for me. I added some specificity $( 'table:visible tbody .check-column input:checkbox' ).click and it sped up to 10-11 ms.

Denis then explained that jQuery processes selectors from right to left. This means that in it's original form it actually gets all checkboxes then throws out the ones that don't have an ancestor with the check-column class, etc. He offered an even more efficient alternative $('table:visible').children('tbody').children('tr.check-column').find('input:checkbox') which processes completely differently by first getting visible tables, then getting their bodies, ... and eventually getting the checkboxes. This method reduced the processing time to 2-3 ms for me.

That's 40-45ms -> 10-11ms -> 2-3ms, which is quite an improvement. It seems like this could be applied to any jQuery selectors that have common elements toward the right side of the selector, especially if the left side of the selector reduces the total amount of nodes that need to be processed.

[Denis-de-Bernardy]

just adding to the previous, the correct selector for that one actually is:

$('table:visible').children('tbody').children().children('.check-column').find('input:checkbox');

(the one aaron mentioned results in an empty array, and it's my bad, not his.) using that brings things to 10ms, from 750ms, on my widgets screen.

[Denis-de-Bernardy]

not finished yet...

[aaroncampbell]

Just a quick note, the latest diff looks like it has the optimizations as well as the logging. I'm not sure if that was intentional.

A huge thanks for this by the way, and if you post when you're done, I'd be happy to test.

[Denis-de-Bernardy]

The figures with the patch applied...

Camino 2.0:

0ms -- adminMenu.init()
14ms -- $('#adminmenu div.wp-menu-toggle').each
4ms -- this.favorites();
4ms -- $('a.separator').click
1ms -- $('body').hasClass('folded')
5ms -- this.restoreMenuState();
0ms -- end adminMenu.init()
1ms -- columns.init()
3ms -- end columns.init()
0ms -- commons.ready()
3ms -- $('.fade').animate
6ms -- $('div.wrap h2 ~ div.updated, div.wrap h2 ~ div.error').addClass('below-h2');
9ms -- $('div.updated, div.error').not('.below-h2').insertAfter('div.wrap h2:first');
4ms -- $('#doaction, #doaction2').click
1ms -- $('#show-settings-link').click
1ms -- $('#contextual-help-link').click
6ms -- $('#contextual-help-link-wrap, #screen-options-link-wrap').show();
9ms -- $( 'table:visible tbody .check-column :checkbox' ).click
5ms -- $( 'thead :checkbox, tfoot :checkbox' ).click
2ms -- $('#default-password-nag-no').click
1ms -- end commons.ready()
0ms -- turbo.ready()
6ms -- end turbo.ready()
1ms -- widgets.init()
1ms -- $('body').hasClass('widgets_access')
4ms -- $('#widgets-right div.sidebar-name').click
3ms -- $('#widgets-left div.sidebar-name').click
2ms -- $('#widgets-right .widget, #wp_inactive_widgets .widget').each
48ms -- this.addEvents();
4ms -- $('.widget-error').parents('.widget').find('a.widget-action').click();
4ms -- $('#available-widgets').droppable
27ms -- $('#widget-list .widget').draggable
169ms -- $('.widgets-sortables').sortable
10ms -- wpWidgets.resize();
11ms -- wpWidgets.fixLabels();
0ms -- end widgets.init()

Safari:

0ms -- adminMenu.init()
10ms -- $('#adminmenu div.wp-menu-toggle').each
7ms -- this.favorites();
8ms -- $('a.separator').click
3ms -- $('body').hasClass('folded')
4ms -- this.restoreMenuState();
2ms -- end adminMenu.init()
0ms -- columns.init()
3ms -- end columns.init()
1ms -- commons.ready()
3ms -- $('.fade').animate
3ms -- $('div.wrap h2 ~ div.updated, div.wrap h2 ~ div.error').addClass('below-h2');
5ms -- $('div.updated, div.error').not('.below-h2').insertAfter('div.wrap h2:first');
2ms -- $('#doaction, #doaction2').click
0ms -- $('#show-settings-link').click
1ms -- $('#contextual-help-link').click
10ms -- $('#contextual-help-link-wrap, #screen-options-link-wrap').show();
5ms -- $( 'table:visible tbody .check-column :checkbox' ).click
3ms -- $( 'thead :checkbox, tfoot :checkbox' ).click
1ms -- $('#default-password-nag-no').click
2ms -- end commons.ready()
1ms -- turbo.ready()
8ms -- end turbo.ready()
1ms -- widgets.init()
2ms -- $('body').hasClass('widgets_access')
2ms -- $('#widgets-right div.sidebar-name').click
1ms -- $('#widgets-left div.sidebar-name').click
2ms -- $('#widgets-right .widget, #wp_inactive_widgets .widget').each
9ms -- this.addEvents();
1ms -- $('.widget-error').parents('.widget').find('a.widget-action').click();
4ms -- $('#available-widgets').droppable
7ms -- $('#widget-list .widget').draggable
89ms -- $('.widgets-sortables').sortable
5ms -- wpWidgets.resize();
3ms -- wpWidgets.fixLabels();
1ms -- end widgets.init()

[Denis-de-Bernardy]

I'll upload a separate patch without the log junk after it gets more testing.

[Denis-de-Bernardy]

the jQuery bug I opened for reference:

​http://dev.jqueryui.com/ticket/4759

[Denis-de-Bernardy]

Widgets screen, with patch applied, is still fully functional on my end. I can add, move, save, close, remove, drop into inactive widgets, move from a sidebar to the next... and I can still remove columns from the editor's screen.

tested in camino 2 and safari 4

it needs FF and IE testing now. I'll get an IE user onto it this WE.

[Denis-de-Bernardy]

these two are with an even larger amount of widgets and checkboxes, on FF 3.5.2 and IE 8 respectively:

​http://pastebin.com/m7bf7bf9d

​http://pastebin.com/m694817c3

this is *after* applying the patch. so, still, a few optimizations to go from the looks of it.

[Denis-de-Bernardy]

my own stats with the very same data as in the previous comment:

Camino - ​http://pastebin.com/m6c719f77

Safari - ​http://pastebin.com/mba59f13

[filosofo]

You're re-arranging deck chairs on the Titanic. jQuery is buggy and inefficient. The UI plugins have been unreliable for years. Every new jQuery release breaks WordPress plugins and themes using older versions. A better solution would be to dump it.

Think of the benefits of having our own WordPress admin JavaScript libraries.

• It could have a reliable, stable API across development cycles
• We could tailor it to meet specific needs. For example here, instead of having to attach event listeners to zillions of widget elements, we could use event delegation.
• We could use progressive enhancement techniques to make the admin more accessible.

[Denis-de-Bernardy]

As much as I agree on principle, the immediate need is to ensure that themes with tons of sidebars (I know I'm not the only one) doesn't crash browsers...

[filosofo]

I understand the short-term need, but I hope to stir up interest in a long-term solution.

[Denis-de-Bernardy]

here's another interesting one... show() and :hidden are extremely slow on slow rendering engines, e.g. Opera 9 with a 400kb widgets page gets:

6ms -- $('#contextual-help-link-wrap')
6071ms -- $('#contextual-help-link-wrap:hidden')

the next calls are very fast, until a new class gets added.

similarly, adding an arbitrary ID to the page will render the next selector that users "#whatever" extremely slow in some engines...

[Denis-de-Bernardy]

by the same token, this:

$( 'table:visible tbody .check-column :checkbox' ).click(...)

is much slower than merely:

$( 'tbody .check-column :checkbox' ).click(...)

[aaroncampbell]

I finally got a chance to fully test in IE 8, FF 3.5.2, Chrome 2, Safari 4, Opera 9.64. In every browser I'm able to drag widgets to sidebars as well as the inactive area, drag widgets between sidebars, edit and save widgets, remove widgets, etc.

The speed differences are definitely measurable, although I wasn't testing with very many widgets or sidebars.

[Denis-de-Bernardy]

improved version of the patch attached. I'm still looking into avoiding to change the widget ID generation.

[azaozz]

Been testing this for a while. For an average widgets page with two sidebars and about 40 widgets on a lower end PC the speed increase is almost unnoticeable:

 Firefox 3.5  195ms -> 125ms
 Safari 4     120ms -> 92ms
 Opera 9.6    830ms -> 640ms
 IE6          1540ms -> 1130ms

(note how browsers without getElementsByClassName are a lot slower). There's only a barely noticeable speed improvement in IE6, 0.4sec. Of course the numbers change in extreme cases: 12 sidebars and 150 widgets containing tables, etc.

Nevertheless this patch is a good idea and would fix some border cases where the widgets page is huge, 1MB+.

The only problem as Denis mentions is that the current patch modifies all internal widgets HTML IDs making them unusable for plugins and breaking any plugin that uses them at the moment. This is done to speed up copying the widget's title from the title field to the widget's header.

There's also a regression there as the title used to be appended when the widget was saved too. Trying some workarounds for this but may eventually need to put the old code back.

[azaozz]

(In [11837]) Speed up jQuery based scripts, props Denis-de-Bernardy, see #10021

[Denis-de-Bernardy]

Sorry for not having followed this much lately. I spotted an issue in my original code.

basically, not all admin screens (in plugins) use:

<form>
<div class="wrap">
<h2>...

some use this instead:

<div class="wrap">
<form>
<h2>...

I'm attaching the needed patch to what was committed.

[Denis-de-Bernardy]

also optimize the sortables

[Denis-de-Bernardy]

10021.5.diff also includes an optimization to the sortables. in opera 9.6, it made quite a difference when I tried it (see the jquery bug I submitted further up)

[azaozz]

(In [11858]) More jQuery based scripts optimizations, props Denis-de-Bernardy, see #10021

[azaozz]

Replying to filosofo:

Think of the benefits of having our own WordPress admin JavaScript libraries...

In principle this sounds good however in practice it would be quite hard to convince plugin and theme authors to learn yet another API. It's hard enough to make them stop bundling jQuery and use the one included in core.

If we switch to our own JS API at least one JS library will still be loaded on every page by a plugin or theme anyways. Also jQuery is getting better, 1.3.3 seems to be coming out soon and has some nice speed improvements.

[filosofo]

Replying to azaozz:

In principle this sounds good however in practice it would be quite hard to convince plugin and theme authors to learn yet another API. It's hard enough to make them stop bundling jQuery and use the one included in core.

The theme and plugin authors can continue to use jQuery if they want, as I'm sure we'd continue bundling it as we do Prototype, for example. But it's not like learning the jQuery API is all that helpful anyways, as it has changed significantly between versions, and we don't have any reason to think the future will be different.

The advantages of a WordPress-built JavaScript API is that it could be truly stable, mainly by focusing on a few things that always need to be done in the WP admin: drag-and-drop, XHR, etc.

If we switch to our own JS API at least one JS library will still be loaded on every page by a plugin or theme anyways.

There's not much we can do to avoid badly-written plugins; they're always going to exist. But the properly-written ones should be loading their libraries only on their own pages, with a few exceptions.

Also jQuery is getting better, 1.3.3 seems to be coming out soon and has some nice speed improvements.

I used to hold out hope for future improvements, too, but jQuery has had plenty of opportunities to address well-known problems and to take advantage of lots of constructive criticism. I see selector speed reports and the like, but they don't mean much when I can turn on Firebug in actual applications and profile, for example, a simple drag-and-drop with tens of thousands of function calls. Or I go open a page on Slashdot and my CPU usage spikes for few seconds while jQuery churns.

[Denis-de-Bernardy]

fix parse error in the script

[Denis-de-Bernardy]

another patch, to fix the parse error that crept in.

[azaozz]

(In [11870]) Fix error in widgets.js, props Denis-de-Bernardy, see #10021

[azaozz]

So I must have tested it with cached js, no wonder that didn't see any difference. Don't see much improvement now too, maybe few milliseconds on an average widgets screen.

[azaozz]

Seems to be working well.

[nacin]

Cross-referencing #11267 for a change in the admin notice animation.