Bug 6863; Use array.join() for string concatenation in getText()

[barberboy]

This fixes jQuery bug #6863: Faster getText by using Array.join('') instead of += for string concatenation.

Based on comparisons of Array.join vs string concatenation on jsPerf this change may have a negative impact on performance in Chrome, Firefox 4 and IE8+. It will have a minor performance benefit in IE6 and IE7, Safari (as of version 5.0.5) and Opera (as of 11.10).

[dmethvin]

Since this patch hasn't eliminated the recursion, the array is likely to be very short (just the child text nodes of the current element plus the text of the recursed elements) and there won't be much benefit compared to flattening out the tree into a single array. Did you perf-test the old vs new getText?

Also, would it be possible for you to take a look at http://bugs.jquery.com/ticket/3144 which is related? The performance differences are nearly as important as the resulting text, and I would love to see that ticket closed. There is a test case in 3144 for the result of new vs old getText.

[barberboy]

Thanks for the feedback, Dave.

No, i don't have a jsPerf for the new vs old getText. And I also realized after reading your comment and looking at the original bug more closely that the request is also suggesting iterating over child nodes using node.nextSibling instead of the childNodes. I had completely missed that.

I'm new to jQuery's collaboration workflow, so could you advise on whether it would be better to make changes/commits to this branch/pull-request, or to close this pull request and create a fresh one after writing a new revision?

Regarding ticket 3144, I'm not familiar enough with the cross-browser inconsistencies of nodeValue vs innerText vs textContent to be too much help there.

[barberboy]

I reverted SHA: 820be06 and rewrote getText to use node.nextSibling-style iteration. I created a jsPerf that demonstrates the performance improvement at http://jsperf.com/sizzle-gettext-bugfix-6863 .

There are a couple of things to note about this implementation:

• The API for Sizzle.getText (as well as jQuery.text) is widened now to accept an Array of DOM elements OR a single Element. This could be noted in the public API or ignored.
• The Array check in this implementation is inferred by an undefined nodeType. This could be changed to be a true array check (toString.call(elems) === "[object Array]"), but it isn't quite as performant as this jsPerf shows: http://jsperf.com/sizzle-gettext-performance/ .

Thanks! I appreciate any feedback.

[timmywil]

We have closed #6863 and #3144.