Forum
Log In
Home
Topic RSS
20:36
10/08/2009
Offline
Hello Tony!
In one project I have jqGrid which show/hide some column dinamically depend on the data loaded. It allows to reduce the number of columns which are needed be displayed. All works OK, but the grid will be slow if the number of rows increase. I analysed the problem and find out that the reason are the line 2855 of the grid.base.js.
I suggest to change the line 2852
and the line 2855 in both cases to the code
The same technique you use already in the most places of the jqGrid code. Such changes increased the total performance dramatically.
Best regards
Oleg
12:51
Moderators
30/10/2007
Offline
Thanks for this Oleg. Fixed.
Tony
For professional UI suites for Java Script and PHP visit us at our commercial products site - guriddo.net - by the very same guys that created jqGrid.
20:33
11/07/2011
Offline
Hi guys,
This fix can be further optimized using this
It doesn't really improve performance (at least not in any noticeable fashion) but it does reduce the number of calls made (since $.css() does a whole bunch of stuff).
22:20
10/08/2009
Offline
I agree, but if we do optimisation we should use this:
Additionally if we do optimisation of the lines we can remove unused j variable from the second .each.
Regards
Oleg
23:47
11/07/2011
Offline
Ha! You totally right, can't believe I missed that. 
I'm gonna update my file right away.
Oh and for even further optimization the each loops should be converted into for loops, I'm gonna try it out on my site and see what kinda difference that makes.
18:09
11/07/2011
Offline
Okay, I did some better testing and here are the method, followed by results.
My data has 337 rows and 3 columns. For every test I reload the page and perform the same action - which is to open the column chooser and hide one column (always the same). To measure performance I used Firebug's console.profile/End() around the $('option',select).each loop in the "apply_perm" function of columnChooser.
I did the test five times for each of the five versions of the code and averaged the times. First version is original. Second is with Oleg's original fix - $(this.cells[i]).css("display", show);. Third one is with further simplfication via this.cells[i].style.display = show;. Fourth is with each loops replaced with for loops in the showHideCol function. Fifth and final one is where I replace the $('option',select).each part with a for loop.
- 24173 calls, average time 451.4ms
- 4453 calls, average time 311.4ms
- 1393 calls, average time 283.4ms
- 1045 calls, average time 279ms
- 1026 calls, average time 277.2ms
Total (from 1 to 5) reduction in number of calls made is almost 24 times, and timing improved by almost 40%. Obviously the biggest improvement was going from 1 to 2, however, considering the triviality of further optimizations I see no reason not to implement all of them.
So the code inside showHideCol, i.e. this
$(this.cells[i]).css("display", show);
});
$($t.rows).each(function(j){
$(this.cells[i]).css("display", show);
});
Can be replaced with this
trows = $($t.rows), trowsL = trows.length, j;
for (j = 0; j < trsL; j++) {
trs[j].cells[i].style.display = show;
}
for (j = 0; j < trowsL; j++) {
trows[j].cells[i].style.display = show;
}
And the code in columnChooser.apply_perm
if (this.selected) {
self.jqGrid("showCol", colModel[this.value].name);
} else {
self.jqGrid("hideCol", colModel[this.value].name);
}
});
Can be replaced with this
for (i = 0; i < ol; i++) {
self.jqGrid(os[i].selected ? 'showCol' : 'hideCol', colModel[os[i].value].name);
}
02:39
10/08/2009
Offline
Small inprovements to your code. You use trows = $($t.rows) and then trows[j].cells[i].style.display = show. Insetead of that one can $t.rows[j].cells[i].style.display = show.
Now my opinion about the usage of for loop instead of .each. I personally use mostly for loop, but it improve the performance only a little, so in my opinion it is mosly the question of the style and not the performance problem. So I use for loop, but never change the code of other people if they use .each.
Now about your suggestion about the columnChooser.apply_perm code. I think the changes bring not so much performane advantages. You can try to mesure to give exact answer. My suggestion would be to rewrite the code so that it uses only one call of showCol and only one call of hideCol. The idea is to use array as the second parameter of showCol and hideCol. For exmaple:var
return colModel[parseInt(this.value,10)].name;
}),
columnsToHide = $('option:not(:selected)', select).map(function () {
return colModel[parseInt(this.value,10)].name;
});
self.jqGrid("showCol", columnsToShow);
self.jqGrid("hideCol", columnsToHide);
instead of the curent code
if (this.selected) {
self.jqGrid("showCol", colModel[this.value].name);
} else {
self.jqGrid("hideCol", colModel[this.value].name);
}
});
I don't mesure, but I think that the usage of on call of showCol and one call of hideCol should improve the performance.
By the way in my project which I change now for one customer I has table with about 56 columns where about 20 columns are visible at the same time. So I would suggest you to increase a little the number of columns if it maks not so many work.
Best regards
Oleg
15:26
Moderators
30/10/2007
Offline
Hello Oleg, 2late2die,
Thank you for the investigations, tests cases and recommendatins.
As 2late2die say the bigest speed improvements is from 1 to 2.
I will see how to implemet them
It is not neccessary to maybe to repace each, since this is not big improvements.
You maybe know this, but the fastest loop operator is while.
We can replace all the code with this one by example
var trs = $("tr",$t.grid.hDiv), trsL = trs.length,
trows = $($t.rows), trowsL = trows.length, j;
while( trsL--) {
trs[trsL].cells[i].style.display = show;
}
while(trowsL--) {
trows[trowsL].cells[i].style.display = show;
}
You can just perform tests 
Regards
For professional UI suites for Java Script and PHP visit us at our commercial products site - guriddo.net - by the very same guys that created jqGrid.
23:11
11/07/2011
Offline
@Tony ahh nice, forgot about the while loop 🙂
I gave it a go (won't bore you with the numbers) but it didn't seem to improve performance noticeably. It did however reduce number of calls further so it's going in, at least for me.
@OlegK good catch on $($t.rows) and the "3" in my post is actually a typo, should've been "13" columns.
Regarding each/for, I agree that the performance improvements are minimal but I prefer them for reduction in the number of calls made - less calls, less chances for something to go wrong 🙂
Now, that "trick" with calling show/hideCol once each is a nice one. I gave it a try and it seemed to have only marginally improved performance, to be fair though, pretty much all results starting with 3 (from my previous post) have fallen within margin of error so in terms of pure ms your original modification is still the only clear and definite improvement.
Having said that, with all the conseqent improvements the number of calls have been reduced from 4453 to 442, which consitutes a 10 fold decrease, so it's all worth while in my opinion. 🙂
BTW, quick question, why did you add "parseInt" in your code?
Anyway, it's now time to tackle setGridWidth as that's the new bottleneck in my code.
Most Users Ever Online: 715
Currently Online:
57 Guest(s)
Currently Browsing this Page:
1 Guest(s)
Top Posters:
OlegK: 1255
markw65: 179
kobruleht: 144
phicarre: 132
YamilBracho: 124
Renso: 118
Member Stats:
Guest Posters: 447
Members: 11373
Moderators: 2
Admins: 1
Forum Stats:
Groups: 1
Forums: 8
Topics: 10592
Posts: 31289
Newest Members:
, razia, Prankie, psky, praveen neelam, greg.valainis@pa-tech.comModerators: tony: 7721, Rumen[Trirand]: 81
Administrators: admin: 66
