I am using the code below that loops around thousands of time, I need this to be as fast as possible, however I am not an expert in Javascript and all I know is that this code is faster than doing it with the innerHTML
method.
Can it be optimized? If so, how?
var a,b,c,d,e,
trEl, tdEl, imgEl
for (i = 0; i < docs.length; i++) {
trEl = document.createElement('tr');
tdEl = document.createElement('td');
td1El = document.createElement('td');
td2El = document.createElement('td');
td3El = document.createElement('td');
imgEl = document.createElement('img');
var str
a = docs[i]['a']
b = docs[i]['b']
c = docs[i]['c']
d = docs[i]['date'] + " - " + docs[i]['timestamp']
e = docs[i]['_e']
trEl.setAttribute('id', e);
trEl.setAttribute('ondblclick', 'scan(' + '"' + e + '"' + ')');
imgEl.setAttribute('src', 'css/bullet.png');
tdEl.appendChild(imgEl);
tdEl.appendChild(document.createTextNode(a));
td1El.appendChild(document.createTextNode(d));
td2El.appendChild(document.createTextNode(b));
td3El.appendChild(document.createTextNode(c));
trEl.appendChild(tdEl);
trEl.appendChild(td1El);
trEl.appendChild(td2El);
trEl.appendChild(td3El);
document.getElementById("search-results").appendChild(trEl)
}
1 Answer 1
Don't evaluate docs.length
in every iteration - it's constant so it only needs to be evaluated once:
for (var i = 0, n = docs.length; i < n; ++i)
Deference docs[i]
into an aliased variable so that you're not repeatedly doing an array lookup into the same element, and don't use strings for keys unless the key is variable:
var doc = docs[i];
var a = doc.a;
...
If the above local variables a
through e
are only used once, don't bother storing them at all, just put them in as trivial parameters when needed (see below)
Use DOM properties instead of setAttribute
function calls when you can:
trEl.id = doc._e; // instead of 'e'
imgEl.src = 'css/bullet.png';
Don't use inline event handlers:
trEl.addEventListener('dblclick', scan, false);
There's no need to pass the variable e
to scan
since it's just the clicked element's ID and therefore trivially extracted within the event handler:
function scan(event) {
var e = this.id;
...
}
Use a DocumentFragment
to store all of the new elements, and then add the entire fragment into the document
in one go, allowing the browser to handle all of the visible DOM changes in one hit:
var rows = document.createDocumentFragment();
for (...) {
...
rows.appendChild(trEl);
}
document.getElementById("search-results").appendChild(rows);
-
\$\begingroup\$ @FiatPax I'd be interested to hear how much performance gain you achieve \$\endgroup\$Alnitak– Alnitak2015年01月07日 16:51:27 +00:00Commented Jan 7, 2015 at 16:51
-
\$\begingroup\$ Here is the report from benchmark.js "optimized x 101,487 ops/sec ±6.66% (56 runs sampled), old x 64,893 ops/sec ±11.52% (64 runs sampled) Fastest is optimized." \$\endgroup\$Fiat Pax– Fiat Pax2015年01月07日 18:21:43 +00:00Commented Jan 7, 2015 at 18:21
-
\$\begingroup\$ I applied all of the methods above except one with event listeners, because if I add trEl.addEventListener('dblclick', scan, false); it will complain its undefined. I'm not sure where to add that code. \$\endgroup\$Fiat Pax– Fiat Pax2015年01月07日 18:22:57 +00:00Commented Jan 7, 2015 at 18:22
-
\$\begingroup\$ That'll depend on where (and how) you've declared the
scan
function. \$\endgroup\$Alnitak– Alnitak2015年01月07日 18:32:23 +00:00Commented Jan 7, 2015 at 18:32 -
\$\begingroup\$ or is it complaining about
addEventListener
being undefined (older MSIE versions), in which case you could just dotrEl.ondblclick = scan
\$\endgroup\$Alnitak– Alnitak2015年01月07日 18:33:50 +00:00Commented Jan 7, 2015 at 18:33