Opened 6 years ago

Closed 3 years ago

#17530 closed enhancement (patchwelcome)

FOR loop performance

Reported by: Samuel Ondrek Owned by:
Priority: undecided Milestone: 1.13
Component: Core Version: 1.9.1
Keywords: Cc:
Blocked By: Blocking:


We should improve a performance of core and files like dojo.js (also other files like dom-class.js, dom-construct.js, dom-form.js, deferred.js or html-js.. and many many others) because they still use slower way of writing loops with checking of length in every iteration. And this is not only performance issue, also semantic and readability side of solition is quite creepy, when we check length in every loop, but inside of loop don't change it.

for(var i=0; i<arr.length; i++) {

for(var i=0, len=arr.length; i<len; i++) {

How slow? .. normally are these impacts not so huge ( in my case 3~5% faster), but if you use Dojo for massive DOM manipulation, it can be really interesting (minimal 10 loops in all dojo files * 5% slower * numberOfElements...).

Minifier business? Also there is situation, that you cannot handle this in Minifier, because sometimes (like in case when you really need every iteration check lenght) you really want to use slower version and it's pretty impossible to detect it..

Google Closure? It's funny, that Dmitry Baranovskiy, author of gRaphael and Raphael, have very critical article about same issue in Google Closure with interview "How to do not javascript" cca 5 years ago!!! (.. and after this article guys from google refactor some code parts).

Change History (5)

comment:1 Changed 6 years ago by bill

You mentioned "massive DOM manipulation"... are you talking about iterating live collections like returned by getElementsByTagName()? If so, we benchmarked that a few years ago and ended up with while(elem = list[idx++]) as the optimal pattern.

Also, since you are claiming a 5% improvement that means that you have a patch and a benchmark? If so, can you submit them as a pull request after signing a CLA?

comment:2 Changed 6 years ago by Samuel Ondrek

Unfortunately I cannot reply specific situations what we are working on (company rules). -- Yep, I can send a pull request, I have signed CLA 2 years ago (I just checked it, looks Ok). -- And no, it doesn't mean that I need patch code, I'm just humble asking with about an idea. It just "nice to have".

comment:3 Changed 6 years ago by Eugene Lazutkin

While I agree with you in general -- in most cases caching an array's length makes perfect sense -- I am a little bit skeptical about the premise: "massive DOM manipulations". I benchmarked the problem several years ago and at that time DOM manipulations were quite heavy -- easily overweighting loop expenses. The problem is that DOM objects are not native JS objects and come with a lot of hidden expenses, so manipulations on numbers with arrays or other intrinsics do not emulate their performance sufficiently. Another thing is that DOM manipulations rarely contain a lot of looping, and frequently it can be avoided by other techniques. Admittedly this benchmarking is long in the tooth now, and we can benefit from new one.

comment:4 Changed 4 years ago by dylan

Component: GeneralCore

comment:5 Changed 3 years ago by dylan

Milestone: tbd1.12
Resolution: patchwelcome
Status: newclosed

Given that no one has shown interest in creating a patch in the past 2+ years, I'm closing this as patchwelcome. Please let us know if you would like to get involved in helping make this change to Dojo!

Note: See TracTickets for help on using tickets.