remove uses of jQuery.each in css module in favor of a for loop

[mikesherov]

as far as I can tell, using $.each is monstrously slow compared to a regular for loop. See here: ​http://jsperf.com/for-loop-vs-each/6

In interest of performance, should we avoid using it internally when possible.

1.8?

[Timmy Willison]

In that perf, you're still talking millions. I don't think we need to worry about the use of $.each defining functions and the like, but we should be concerned about the performance of the most commonly used functions (and I think this is already done internally for the most part).

[Timmy Willison]

-1, I'm not against performance improvements, but we should balance performance with size, maintainability, and readability.

[mikesherov]

timmywil, to your point, there are only a few places where .each is used in actual function execution, and one of them is css.js. Can we change the title to REMOVE USES OF JQUERY.EACH INTERNALLY IN FAVOR OF A FOR LOOP WHEN IMPACTING FUNCTION EXECUTION

I'll adress each point for just that example: size: replacing jQuery.each with a for loop in getWH(), the code when gzipped and minified is one byte less. readability: is a for loop less readable than $.each? Maybe. maintainability: is a for loop less maintainable than $.each? Maybe. perf: no choice but to be faster, but I'll make a perf to prove it. It might be negligible.

If the perf is negligible and the savings is only 1 byte, than perhaps $.each is more readable enough to warrant not changing.

Thanks for the response.

[Timmy Willison]

@mikesherov: actually, I'd be in favor of the change in getWH. That is something that does get used a lot and possibly called many times per second in certain use cases.

[Timmy Willison]

Please feel free to present more spots you think we could benefit from this change. Changing my vote to +0, depending on the place.

[mikesherov]

Ok. Will do.

[mikesherov]

So, it seems that .each is used mostly for function definition except for:

• effects.js:
  • the .step() function when done to restore offset properties.
  • the genFx function which seems to get called once per animation
• manipulation.js
  • when eval'ing scripts in .domManip()
• css.js
  • where I already mentioned

Seems like only in .css is worth it. Please change ticket name to reflect only in .css. I'll submit a perf and PR shortly.

[mikesherov]

savings minimal: ​http://jsperf.com/each-vs-for-loop-in-css-js

[Rick Waldron]

Not surprised that modern browsers are just fast. What about IE6, 7?

[mikesherov]

I don't have IE7 and IE6 on my laptop, anyone else willing to test it in IE7 and IE6 for me?

[mikesherov]

IE7 seems negligible. IE8 / safari / opera / and FF seem to have decent speed ups, on the the order of 10 - 20%.

I'll submit the PR, and leave it up to you guys on whether it's less maintainable or not.

[Timmy Willison]

Sounds good to me.

[Rick Waldron]

@mikesherov say hello to your new BFF: ​http://www.browserstack.com

[mikesherov]

​https://github.com/jquery/jquery/pull/590

[mikesherov]

please close this ticket as the PR has been merged.

[jaubourg]

Replying to mikesherov:

please close this ticket as the PR has been merged.

Mike, use "Fixes #XXX" where XXX is the id of the ticket in your commit message next time and the ticket will be closed automagically ;)

[mikesherov]

yeah, I missed on that one.

[Timmy Willison]

OR, if dave used pulley... ;)