Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

each Chrome performance #827

Closed
ghost opened this issue Feb 24, 2015 · 11 comments
Closed

each Chrome performance #827

ghost opened this issue Feb 24, 2015 · 11 comments

Comments

@ghost
Copy link

ghost commented Feb 24, 2015

I get horrible performance from $.each() in Chrome.

Can I request that it no longer be used for new loops?

If's it's alright, I'll push some conversions for the old ones.

Thanks.

@Mottie
Copy link
Owner

Mottie commented Feb 24, 2015

Hi @jimbobcoder!

Sure! I would appreciate any help :)

@Mercuenomia
Copy link

Count me in.

Side note: thank you very much for this library. Lifesaver.

Let me know if you need help animating Pager.

@Mottie
Copy link
Owner

Mottie commented Feb 24, 2015

I was looking through the code, and really the only place it looks like I use $.each() is for small arrays, like processing selected widgets and updating headers. Usually arrays the same size as the number of columns in the table. The only code that looks like it might show a noticeable benefit would be the pager code - creating options for the page selector & calculating the number of filtered rows.

I'm all for performance enhancement, but I don't see where in the current code there would be a major issue.

Don't get me wrong, I still want to change things over to plain javascript. I was looking at this jspref and found it surprising that $.each() is faster than for...in and array.forEach, but still much slower than a for loop.

@ghost
Copy link
Author

ghost commented Feb 24, 2015

Yes, it looks like it could be finished relatively quickly since they're compact tight loops.

For in must execute a function or Chrome will not optimize. I can't speak on foreach.

Sampling the length once and looping via index is the fastest, I've found. $.each previously used just about anything, so that could explain its middling type results. They may be optimizing their fors, hopefully.

The optimizer identifies trigger for me.

@Mottie
Copy link
Owner

Mottie commented Feb 26, 2015

I took care of the core plugin, filter widget and pager (addon & widget) so far.

I'll try to get to the other widgets soon.

@Mercuenomia
Copy link

Thanks! My tables are even faster now!

@ghost
Copy link
Author

ghost commented Feb 26, 2015

Wow, that was fast.

Would you like me to convert the eaches on the jQuery objects?

@Mottie
Copy link
Owner

Mottie commented Feb 26, 2015

From that jsPref test, it seems the jQuery each is faster with objects. I don't know how that is possible, but most of the remaining .each()'s are on objects.

@ghost
Copy link
Author

ghost commented Feb 26, 2015

For smaller sizes, it look like they perform equally.

For larger sizes, it looks like there's still a large percentage increase in performance converting to an index loop.

Which revision shows each as faster?

@Mottie
Copy link
Owner

Mottie commented Feb 26, 2015

The one I linked before...

What I was trying to say is that there are no large sizes. The largest object that is being run through the $.each() would match the number of columns in a table. So unless you have a table with 500 columns, I don't think you would notice a differece.

@ghost
Copy link
Author

ghost commented Feb 26, 2015

I just tested, and the compiler is still complaining that it cannot optimize the object eaches "ForInStatement is not fast case", so it looks like jQuery is using the old for in without wrapping the contents in a function.

Easy fix might be to wrap the contents of the object eaches in functions? I'll try and then get back to you as soon as I've tested.

@Mottie Mottie closed this as completed in 8d2abbb May 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants