current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite
1

I have a rather large javascript array of objects that I am transforming into an HTML table. Each object in the array looks like:

{
    name:'Document Title',
    url:'/path/to/document.html',
    categories:['Some Category'],
    protocols:['ID Number','Another ID'],
    sites:['Location']
}

It's possible to have multiple categories, protocols, and sites, hence why they are in arrays.

The jQuery I've written to do the transformation is:

var table = $('<table class="table table-bordered table-condensed table-striped data-table">'),
    tbody = $('<tbody>'),
    thead = $('<thead><tr><th width="100">Protocol</th><th>Report Name</th><th width="140">Category</th><th width="220">Site</th></tr></thead>');

$.each(reports,function(i,report) {
    var tr = document.createElement('tr'),
             cellProtocol = document.createElement('td'),
             cellName = document.createElement('td'),
             cellCategory = document.createElement('td'),
             cellSite = document.createElement('td'),
             reportLink = document.createElement('a');

    // If property doesn't exist, set to empty array
    report.protocols = report.protocols || [];
    report.sites = report.sites || [];
    report.categories = report.categories || [];

    reportLink.setAttribute('href',report.url);
    reportLink.appendChild(document.createTextNode(report.name));

    cellProtocol.appendChild(document.createTextNode(report.protocols.join(', ')));
    cellName.appendChild(reportLink);
    cellCategory.appendChild(document.createTextNode(report.categories.join(', ')));
    cellSite.appendChild(document.createTextNode(report.sites.join(', ')));

    tr.appendChild(cellProtocol);
    tr.appendChild(cellName);
    tr.appendChild(cellCategory);
    tr.appendChild(cellSite);

    tbody[0].appendChild(tr);
});

$('#report-table').empty().append(table.append(thead,tbody));

It currently takes about 2 seconds to transform 4200 objects into table rows and display the finished table. In the $.each loop I am using appendChild as I understand it is faster than using innerHTML or $.append.

Are there any improvements here I can make in performance?

share|improve this question
    
Did you take a look at documentFragment (which is known to be faster) ? Usage : developer.mozilla.org/en/DOM/document.createDocumentFragment and article talking about it : ejohn.org/blog/dom-documentfragments​​ –  dievardump Jul 19 '12 at 20:09
    
You didn't mention which browser. quirksmode.org/dom/innerhtml.html (which, I know, it's old) would imply that you need to use innerHTML to load all of your content. But using the handy tests on the page, I found that using DOM node creation, etc. was fastest in Chrome. –  JayC Jul 20 '12 at 18:29
1  
$.each is actually quite slow too jsperf.com/for-vs-each-vs-each/3 –  George Mauer Jul 21 '12 at 3:01
    
A jsPerf would be good for something like this. –  Inkbug Jul 23 '12 at 5:26
    
Repeating jsanc623's remark: Why not do the HTML server-side? As the matter of the fact: Why are you using AJAX here in the first place? –  RoToRa Jul 23 '12 at 12:14

2 Answers 2

up vote 2 down vote

If this is on an intranet, with similar machines running this JS, then 2 seconds is fairly ok for 4,200 records...but if its on the internet, you might want to think about doing this server side and just ajax'ing the server generated table code via js...someone running this on an iPad would probably take considerably longer to generate 4,200 records vs someone running an overclocked i7 machine for instance. Just food for thought.

As for actual improvements to the code, I don't really see any glaring items...although someone else might want to pipe in. Seems someone already piped in :)

share|improve this answer
    
Valid point; this timing was local on my own development machine. Potential end users include outside users accessing it via the internet. –  jackwanders Jul 20 '12 at 18:34
    
@jackwanders - which would add to the time it would take to parse everything...if a node somewhere between your system and the user is overloaded - it's going to take a whole lot more than 2 seconds, especially considering most sites take that long to load nowadays. –  jsanc623 Jul 23 '12 at 15:37
up vote 0 down vote

This looks like something you'd want to do server-side, mainly because you can cache the result. So for every request but the first one you're essentially just downloading a file.

If you can't do it server-side, here are a few improvements:

  • Ditch jQuery. If you are looking for performance you don't want to be doing dozens of checks for bugs in browsers that you don't support anyway.
  • Don't use forEach, each, etc - that's the slowest possible way to loop through the objects. Instead try for (i = 0; report = reports[i]; i++), which will be performant on an array this large. Or, more simply, for (i = 0, len = reports.length; i < len; i++), because most browsers have optimized the hell out of it.
  • Ensure that the objects always have categories, protocols, and sites properties, even if they are empty arrays, so that you don't have to waste time filling in defaults. Alternatively, if there are a large number of empty arrays and sending them all would be a waste of bandwidth, you can still optimize this by skipping the default checks and replacing report.protocols.join(', ') with report.protocols ? report.protocols.join(', ') : '', which avoids creating unnecessary arrays.
  • This is very minor, but caching globals could help. So, var createElement = document.createElement;. There is a catch though, which is that browsers have heavily optimized built-in objects like document and Math, and in some cases this doesn't carry over to local pointers. So, strangely enough, in some browsers a local pointer to document will actually be slower than the global pointer to document. But I've found that this is not usually the case, and therefore caching globals is still an optimization even when the globals are built-in objects.
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.