views:

52

answers:

2

Before I dive in to the jQuery/Sizzle source I thought i'd ask here about ways to speed the below method up.

This is a standard "Check All" checkbox scenario. A header cell (<thead><tr><th>) has a checkbox that when checked checks all other checkboxes in it's table's tbody that are in the same column.

This works:

// We want to check/uncheck all columns in a table when the "select all"
// header checkbox is changed (even if the table/rows/checkboxes were 
// added after page load).
(function () {
    // use this dummy div so we can reattach our table later.
    var dummy = $("<div style=display:none />");

    // hook it all up!
    body.delegate(".js_checkAll", "change", function () {

        // cache selectors as much as possible...
        var self = $(this),
            // use closest() instead of parent() because 
            // the checkbox may be in a containing element(s)
            cell = self.closest("th"),
            // Use "cell" here to make the "closest()" call 1 iteration 
            // shorter. I wonder if "parent().parent()" would be faster 
            // (and still safe for use if "thead" is omitted)?
            table = cell.closest("table"),
            isChecked,
            index;

        // detaching speeds up the checkbox loop below.
        // we have to insert the "dummy" div so we know
        // where to put the table back after the loop.
        table.before(dummy).detach();

        index = cell.index();
        isChecked = this.checked;

        // I'm sure this chain is slow as molasses
        table
            // get only _this_ table's tbody
            .children("tbody")
            // get _this_ table's trs
            .children()
            // get _this_ table's checkboxes in the specified column
            .children(":eq(" + index + ") :checkbox")
            // finally...
            .each(function () {
                this.checked = isChecked;
            });

        // put the table back and detach the dummy for
        // later use
        dummy.before(table).detach();

    });
} ());

However, for 250+ rows, it starts to become slow (at least on my machine). Users may need to have up to 500 rows of data so paging the data isn't the solution (items are already paged @ 500/page).

Any ideas how to speed it up some more?

+2  A: 

I wouldn't use all those calls to .children() like that. You'd be much better off just using .find() to find the checkboxes, and then check the parents:

table.find('input:checkbox').each(function(_, cb) {
  var $cb = $(cb);
  if ($cb.parent().index() === index) cb.checked = isChecked;
});

By just calling .find() like that with a tag name ('input'), Sizzle will just use the native getElementsByTagName (if not querySelectorAll) to get the inputs, then filter through those for the checkboxes. I really suspect that'd be faster.

If finding the parent's index gets expensive, you could always precompute that and store it in a .data() element on the parent (or right on the checkbox for that matter).

Pointy
+1  A: 
// I wonder if "parent().parent()" would be faster 
// (and still safe for use if "thead" is omitted)?

No. If <thead> is omitted then in HTML a <tbody> element will be automatically added, because in HTML4 both the start-tag and the end-tag are ‘optional’. So in HTML, it would be parent().parent().parent(), but in XHTML-served-as-XML, which doesn't have the nonsense that is optional tags, it would be parent().parent().

It's probably best to stick with closest(). It's clearer, it's not particularly slow and you're only using it once, so it's not critical anyway.

index = cell.index();

Although, again, this is only once per table so not critical, there is a standard DOM property to get the index of a table cell directly, which will be faster than asking jQuery to search and count previous siblings: index= cell[0].cellIndex.

// we have to insert the "dummy" div so we know
// where to put the table back after the loop.

That's a bit ugly. Standard DOM has a better answer to this: remember the element's parentNode and nextSibling (which may be null if this is the last sibling) and when you're done you can parent.insertBefore(table, sibling).

        .children("tbody")
        .children()
        .children(":eq(" + index + ") :checkbox")
        .each(function () {
            this.checked = isChecked;
        });

You should consider using .children().eq(index) rather than hiding that away in a selector. Won't make a big difference, but it's a bit clearer.

In any case, you can save jQuery's selector engine a bunch of work by using some more standard DOM to traverse the table:

$.each(table[0].tBodies[0].rows, function() {
    $(this.cells[index]).find('input[type=checkbox]').each(function() {
        this.checked = isChecked;
    });
});

Selector queries can be fast, when they're operating against the document and using only standard CSS selectors. In this case jQuery can pass the work onto the browser's fast document.querySelectorAll method. But scoped selectors (find and the second argument to $()) can't be optimised due to a disagreement between jQuery and Selectors-API over what they mean, and non-standard Sizzle selectors like :eq and :checkbox will just get rejected. So this:

$('#tableid>tbody>tr>td:nth-child('+(index+1)+') input[type=checkbox]')

could actually be faster, on modern browsers with querySelectorAll!

bobince
Even without querySelectorAll, just getting all the `input` elements in the table would by pretty fast via `getElementsByTagName`, no?
Pointy
It would but I don't know if that's what the OP wants. Judging by the efforts gone into getting the column index and avoiding nested tables I would guess maybe there are other inputs/checkboxes/tables in there somewhere whose contents need to be avoided. If not, then yeah, `$(table[0].tBodies[0]).find('input')`, using `getElementsByTagName` would indeed be fastest.
bobince
Right - well if there are indeed lots of columns of checkboxes, then the best thing to do would be to tag the checkboxes with a class at page generation time, or, failing that, do it once when the page loads.
Pointy
thanks, here is what I did with your advice: http://jsbin.com/ogoho4
David Murdoch
Cool. Have you tested it with bigger tables and found any significant speed difference? To be honest I'm not convinced there's enough speedup there to make it worth maintaining two branches of code for this.
bobince
I had actually made a test page on jsbin w/ benchmarks...while testing the test page I accidentally created an infinite loop that forced me to "hard" shut-down my machine. Apparently Chrome dev channel doesn't have the log-running-script timeout message.Anyway, querySelectorAll is about 40% faster in Chrome 8.0.552.18 dev for large tables. One of the tables I am using it on is quite "heavy" with images, nested tables, sortable columns, etc.
David Murdoch