After pondering and reviewing all the talks held on the "Performance Tweaks" thread, I rewrote the patch to the "merge()" method by using lazy functions. The real gain is obtained by doing the browser check only once instead of every time.

By not using the "new Function" constructor, as John suggested, the code will remain compatible with AIR. This method is called thousand of times in intensive selector queries so I tuned a bit the loop trying to avoid to lookup in the same array several times.

For IE I left the check on nodeType != 8 instead of nodeType == 1 as was suggested by me and Matt Kruse because this part is not really related to the "Performance Tweaks" we were talking.

Diego Perini

At the very beginning of the merge function I read this comment:

// We have to loop this way because IE & Opera overwrite the length
// expando of getElementsByTagName

Maybe that's the reason to stay with push to add elements to the array and not to use first.length, even if it is not the best choice about performance

I would still like to recommend this change instead:

jQuery.merge = function (first,second) {
  var d;
  if (document.createElement && document.createComment) {
    d = document.createElement('div');
    d.appendChild(document.createComment("test"));
    if (d.getElementsByTagName && d.getElementsByTagName('*').length>0) {
      return (newMerge3=function(first,second) {
        var i=0,e;
        while(e=second[i++])
          if (e.nodeType!=8)
            first[first.length]=e;
        return first;
      })(first,second);
    }
  }
  return (newMerge3=function(first,second) {
    var i=0,e;
    while(e=second[i++])
      first[first.length]=e;
    return first;
  })(first,second);
}

It has the advantage of being a speedup and also avoiding the unnecessary browser sniff.

See http://groups.google.com/group/jquery-dev/browse_frm/thread/5019fa537c8d3595/3fb717b0dc43e4f0

Sorry, I had left the "newMerge3" reference inside the code...

jQuery.merge = function (first,second) {
  var d;
  if (document.createElement && document.createComment) {
    d = document.createElement('div');
    d.appendChild(document.createComment("test"));
    if (d.getElementsByTagName && d.getElementsByTagName('*').length>0) {
      return (jQuery.merge=function(first,second) {
        var i=0,e;
        while(e=second[i++])
          if (e.nodeType!=8)
            first[first.length]=e;
        return first;
      })(first,second);
    }
  }
  return (jQuery.merge=function(first,second) {
    var i=0,e;
    while(e=second[i++])
      first[first.length]=e;
    return first;
  })(first,second);
}

Replying to rformato:

At the very beginning of the merge function I read this comment: {{{ // We have to loop this way because IE & Opera overwrite the length // expando of getElementsByTagName }}} Maybe that's the reason to stay with push to add elements to the array and not to use first.length, even if it is not the best choice about performance

After a search on svn I found [1594], so the comment was referring to the second array, not the first. The patch is good.

Also consider storing first.length out of the loop in a local var and incrementing it on array augmentation:

jQuery.merge = jQuery.browser.msie ?
             function( first, second ) {
                 var a, i = -1,fl = first.length;
                 while (a = second[++i])
                     if ( a.nodeType != 8 )
                         first[fl++] = a;
                 return first;
             } :
             function( first, second ) {
                 var a, i = -1,fl = first.length;
                 while (a = second[++i])
                     first[fl++] = a;
                 return first;
             };
return jQuery.merge( first, second );

This will speedup merge of another 5% in IE, much less in other browsers but it won't harm in any case

Added the looping optimizations at [5578], the code bifurcation will need to be approved in a general way for the code.