-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
improve transition performance #670
Conversation
src/shared/transitions.js
Outdated
}) | ||
.concat(id + ' ' + duration + 'ms linear 1 forwards') | ||
.join(', '); | ||
while (i--) hash = (hash * 33) ^ str.charCodeAt(i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 33 vs something like 31?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that this is copied from the string-hash library. 31
is better than 33
because it is a prime that will give the same distribution as other primes like 33
, but it can also be optimized into a bitshift instead of multiplication by the VM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
primes like
33
😑 💥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this out on esbench and there doesn't seem to be anything in it — 33 is usually faster but not consistently. There's very little in it either way. (I thought bitshifting only worked with powers of two? Though I freely admit I don't really know what I'm talking about 🙃 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, how did I actually think 33 is a prime number... anyways 33 is bad because it's also not a prime number.
This is from the book Effective Java 2nd Edition which is a very good book:
The value 31 was chosen because it's an odd prime. If it were even and multiplication overflowed, information would be lost, as multiplication by 2 is equivalent to shifting. The advantage of using a prime is less clear, but it is traditional.
A nice property of 31 is that the multiplication can be replaced by a shift and subtraction for better performance:
31 * i == (i << 5) - i
Modern VMs do this sort of optimization automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, whaddaya know — if you do the bitshifting yourself, you get a significant speedup!
function bitshift33 (str) {
var hash = 5381;
var i = str.length;
while (i--) hash = ((hash << 5) + hash) ^ str.charCodeAt(i);
return hash >>> 0;
}
I.e. it's a shift and addition rather than shift and subtraction.
According to this (which https://github.com/darkskyapp/string-hash is based on) 33 does give better results than other numbers, including primes, and no-one is quite sure why. So I'll update this PR to use manual bitshifting, but I reckon we might be better off sticking with 33?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no evidence that 31 or 33 is better because I don't actually know how that works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay wait, I think I see why 31 is better. 31
in base 10 is actually 11111
in base 2. That means by shifting it over 5, you get 1111100000
. So then when you subtract (or add) a number you get a more even distribution because there's a better chance of a bit being changed by the operation.
This PR substantially improves framerate when there are lots of CSS transitions in a document with a combination of techniques:
<style>
tag for each transition, we just create the one, lazily, and update itstylesheet.insertRule(rule, index)
rather thanstyle.textContent = rule
<style>
tag once there are no active transitions — i.e. when frame drops caused by updating the CSSOM are less likely to be noticed