Improve IE perf \o/ #1

Closed
jdalton opened this Issue Jul 29, 2013 · 7 comments

Comments

Projects
None yet
2 participants
@jdalton

jdalton commented Jul 29, 2013

A while ago you tweeted that IE11 perf was lacking.
After some investigation it looks like the issue is on line 44 of screen.js.

If you change

var i = (y * imagedata.width + x) * 4;

to this

var i = (y * (imagedata.width | 0) + x) * 4;

you'll get better performance.

The issue comes down to Chakra incorrectly type specializing i as a float, partly because it lacks information about the DOM property imagedata.width. Explicitly coercing imagedata.width to an integer gives Chakra the info it needs to avoid the mix-up. I've filed a bug with Chakra to address this issue.

/@dstorey

@thibaultimbert

This comment has been minimized.

Show comment
Hide comment
@thibaultimbert

thibaultimbert Jul 29, 2013

Owner

Hi JD,

Thanks a lot for following up on this. I will give it a try today for sure.

Thibault

Owner

thibaultimbert commented Jul 29, 2013

Hi JD,

Thanks a lot for following up on this. I will give it a try today for sure.

Thibault

@thibaultimbert

This comment has been minimized.

Show comment
Hide comment
@thibaultimbert

thibaultimbert Jul 29, 2013

Owner

Just made the change, huge performance difference. Thanks again. Will probably blog about this today.

Owner

thibaultimbert commented Jul 29, 2013

Just made the change, huge performance difference. Thanks again. Will probably blog about this today.

@jdalton

This comment has been minimized.

Show comment
Hide comment

jdalton commented Jul 29, 2013

dance

@thibaultimbert

This comment has been minimized.

Show comment
Hide comment
@thibaultimbert

thibaultimbert Jul 29, 2013

Owner

Haha, I shot a little video showing before/after: https://vimeo.com/71271108

Owner

thibaultimbert commented Jul 29, 2013

Haha, I shot a little video showing before/after: https://vimeo.com/71271108

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Jul 29, 2013

As a side note I've also seen issues with snippets like this

for (var i = 0; i < canvas.height * canvas.width; i++) {

needing to be changed to

var area = canvas.width * canvas.height;
for (var i = 0; i < area; i++) {

to avoid blind spots and increase performance.

You probably can't go wrong with hoisting canvas property values out of loops and coercing them to integers:

var area = (canvas.width * canvas.height) | 0;

jdalton commented Jul 29, 2013

As a side note I've also seen issues with snippets like this

for (var i = 0; i < canvas.height * canvas.width; i++) {

needing to be changed to

var area = canvas.width * canvas.height;
for (var i = 0; i < area; i++) {

to avoid blind spots and increase performance.

You probably can't go wrong with hoisting canvas property values out of loops and coercing them to integers:

var area = (canvas.width * canvas.height) | 0;
@thibaultimbert

This comment has been minimized.

Show comment
Hide comment
@thibaultimbert

thibaultimbert Jul 29, 2013

Owner

Yes, hoisting is an optimization I always try to do for loops. I never thought of the explicit coercion, even though asm.js relies on it. Thanks for the great reminder.

Owner

thibaultimbert commented Jul 29, 2013

Yes, hoisting is an optimization I always try to do for loops. I never thought of the explicit coercion, even though asm.js relies on it. Thanks for the great reminder.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Jul 29, 2013

Yes, hoisting is an optimization I always try to do for loops.

Though if you were to replace canvas with a vanilla JS object containing width and height properties there shouldn't be an issue, so this is really a canvas/dom blind spot and not a blanket gap.

jdalton commented Jul 29, 2013

Yes, hoisting is an optimization I always try to do for loops.

Though if you were to replace canvas with a vanilla JS object containing width and height properties there shouldn't be an issue, so this is really a canvas/dom blind spot and not a blanket gap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment