Optimizations for the merge() method

[diego]

By using the Firebug profiler I realized that the "merge()" function is one of the most often called function during element selection or matching and is the second most consuming function in a slickspeed run (7880 calls 27.6% time jquery 1.2.3.js, linux FF 2.0.12).

These are the reasons for which I thought this method may be improved:

- IE is checked for each call, even if we are using other browsers
- the loop is looking up the same array twice instead of just once
- there is a written (only) duplication of the for loop

Take the last less seriously...but only one executes and cross browser debugging requires two hand written break-points...ha.ha.ha...

Probably this has been exacerbated by my last works ;-)

However even if it looks ugly what it does is simple, a function for each browser is compiled runtime when the absolute first call hit the function, and this is also the most compact way of writing it, well after removing the unnecessary concatenation fat.

    merge: function( first, second ) {
        // We have to loop this way because IE & Opera overwrite the length
        // expando of getElementsByTagName
        // Also, we need to make sure that the correct elements are being returned
        // (IE returns comment nodes in a '*' query)
        this.merge = new Function('first, second',
                'var a, i = -1;'+
                'while (a = second[++i])'+
                    (jQuery.browser.msie ? 'if ( a.nodeType != 8 )' : '')+
                    'first[first.length] = a;'+
                'return first;'
        );
        return this.merge( first, second );
    },

on the thread we can probably look at less eye intrusive version.

Please tell if I should build/attach a real diff once this has been reviewed.

One catch is that after this, merge() shows up as an anonymous function, don't know if jQuery use some trick on fn names, however it worked and now slickspeed numbers are different and looking at the profiler again have...7880 calls 17.8% time.

Well surely need review...gain start to be suspiciously high... did I miss the obvious ?

Diego Perini

[diego]

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

[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

[mkruse]

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

[mkruse]

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);
}

[rformato]

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

[flesler]

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

[ammy]

• Web Design * SEO Services * Free Website Builder * PSD To HTML * Ecommerce Web Development * Search Engine Optimization * Web Design Services * Web Designing * Web Development * Custom Web Design * SSL Certificates

[aasin]

Great Work!

RapidSSL Certificates * VeriSign SSL * GeoTrust SSL * Thawte SSL * SSL Certificate

[casininiox]

casino onlineè tutelato dal diritto d'autore casino online Offre la possibilità di registrarsi per creare e gestire gratuitamente il proprio europa casino

[casininiox]

casino online Vieni e Gioca al Vero Casinò! Scegli il Tuo Gioco Preferito. casino online europa casinoLa prestigiosa destinazione di gambling online

[kevin1]

* St Pete Computer Repair * Computer Service St Petersburg * Computer Repairing St Petersburg