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

Performance of setStyle #2679

Open
oallouch opened this issue Nov 25, 2014 · 4 comments
Open

Performance of setStyle #2679

oallouch opened this issue Nov 25, 2014 · 4 comments
Milestone

Comments

@oallouch
Copy link

oallouch commented Nov 25, 2014

Hi everyone,
In modern web (or mobile) applications, setStyle can be called millions of times. It's, for me, the most crucial code for good lower level performance.
So, optimization ideas for the Element.setStyle function:
. remove the camelCase(). I've already done it in my version. Try it ! It's great :)
. remove the Math.round around 'val'. With retina screens, sub-pixels become real pixels
. (Element.Styles[property] || '@').split(' ') should be cached. Instead of Strings, Element.Styles values should be Arrays
. if the split array has a length of 1, we don't have to do a "Array.from(value)"

What do you think of it ?

Thx,

Olivier Allouch
http://www.illicotravel.com

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/6506644-performance-of-setstyle?utm_campaign=plugin&utm_content=tracker%2F22067&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F22067&utm_medium=issues&utm_source=github).
@SergioCrisostomo
Copy link
Member

@oallouch nice input!
We will be dropping some old browsers soon and that will make possible some nice refactoring. If you want to take part on that would be great. I can ping you here.

@oallouch
Copy link
Author

I don't have plenty of time (3 parallel projects), but always for mootools !

@ibolmo ibolmo added this to the 1.6.0 milestone Jan 14, 2015
@DimitarChristoff
Copy link
Member

  • removing the camelCase is breaking, 👎 - code in the wild references properties in the stupidest way possible, mix of scripting, CSS styles, prefixes etc. although I agree this is going to help, it will break people's apps. perhaps can be protected by compat mode.
  • rounding 👍 - for https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L125 - though sub pixel rendering is fine, this code path only gets hit if you fail to specify your own unit so it needs to revert to the map - or you are passing an array of multiple values. you can get more performance avoiding the lookup and rounding by just adding the units yourself. the other usecase https://github.com/mootools/mootools-core/blob/master/Source/Element/Element.js#L128 is actually hit when you pass in your own string that's just a number, eg String(300.5) would turn to 301, not sure why is is being done--somebody would know. arguably again, if you pass 300.5px, you will hit the fastest code path where it does not have to do anything once it determines it's a string that does not coerce to number and will just set the style property.
  • Element.Styles could use arrays indeed, and that will help - but it's going to break stuff again due to a lot of code that adds to the styles collection for custom property parsing, animating transitions and CSS3 props mootools does not 'know about'. Right now you can add custom parsers by simply doing Element.Styles.textShadow = '@px @px @px @'; document.getElement('.foo').setStyle('text-shadow', [2, 2, 2, 'red']); - in the new syntax, this would be more like ['@px', '@px', '@px', '@'] which is slightly awkward - Array.from(value) is a little defensive and can probably go. caching and pre-emptive split is not an option that will work very well due to late adding of element styles. your gain will be lost by looking to see if the cache is populated or splitting afresh after...

to be honest you don't need to maintain your own version of mootools for this - just add Element.prototype.css = function(key, value) { .. your faster one } and use it like jQuery. If you change styles all the time you are probably animating - and if performance is your concern, look at velocity.js as it is cleaner and perf-centric style changes / tweens

@oallouch
Copy link
Author

It's the camelCase that hurts most, but, you're right, it comes from an age when we didn't have css animations.
Thx for the velocity tip. Looks great.

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

4 participants