IE11 polyfill performance
4nderss opened this issue · 19 comments
I'm submitting a bug report
- Library Version:
1.1.1
Please tell us about your environment:
-
Operating System:
Windows [10] -
Node Version:
5.9.1 -
NPM Version:
3.10.5 -
Browser:
IE 11 -
Language:
all
Current behavior:
In Chrome the collection,js polyfill is not needed/used. Our site renders in ~1000ms. In IE11, where the polyfill is used, our site renders around 400% slower, around ~4500ms. While trying to profile IE11 and see what is taking such a long time, we see the function has() from collections.js being called over 1000 times with ~20-30ms cpu time.
https://github.com/4nderss/aurelia-performance-test
Expected/desired behavior:
IE11 should not be 400% slower than modern browsers using aurelia-polyfills.
-
What is the expected behavior?
Lower performance is expected, but not 400%. -
What is the motivation / use case for changing the behavior?
Better performance
Is there any way to replace aurelia-polyfill with another like core-js?
I have written a small application that demonstrates this exact issue
https://github.com/4nderss/aurelia-performance-test
I get 2592 ms in chrome and 25000 ms in IE11(with profiler).
@jdanyow Any chance you can take a look into this?
I think your properties property is killing your performance. Every time it's accessed it allocates a new array instance and needs to loop over the props in a user object to fill the array:
import {HttpClient} from "aurelia-fetch-client";
import {inject, computedFrom, TaskQueue} from "aurelia-framework";
@inject(HttpClient,TaskQueue)
export class App {
constructor(private client: HttpClient, private taskQueue: TaskQueue) {
client.configure(config => {
config
.useStandardConfiguration()
});
}
message = 'Hello World!';
@computedFrom("users")
get properties() {
let res: string[] = [];
let object = this.users[0];
for (var property in object) {
if (object.hasOwnProperty(property)) {
res.push(property);
}
}
return res;
}
users: any[] = [];
activate() {
//see that data is loaded before we start
return this.client.fetch('https://api.github.com/users')
.then(response => response.json())
.then(users => {
let res = users as any;
while(this.users.length < 1000) {
for (var index = 0; index < res.length; index++) {
var element = res[index];
this.users.push(element);
}
}
this.performanceTime = performance.now();
});
}
performanceTime: number;
attached() {
this.taskQueue.queueTask(() => {
this.performanceTime = (performance.now() - this.performanceTime) | 0;
});
}
}Change it to a method: createPropertiesArray and change the last line, return res to this.properties = res;. Finally, call createPropertiesArray at the end of your activate method.
Oh if it were that simple.. I'm sorry to say but that array was just for this perticular test case and is not what we use in our application code. I have updated my example github project and the performace is still terrible. Around 10 000 ms average for IE11/Win 10 (no improvmement at all). 25000 ms with profiler running.
So you can go ahead and re-open this issue if this is something aurelia intend to adress (if even possible?). I know IE11 does not have that big of a market share, but for businesses I beleive it is still one of the most commonly used, atleast for our userbase.
If aurelia can't fully support IE11 there should be some kind of information regarding this so that other developers doesn't make the same mistake we did. Right know it looks like we have to swap our aurelia templates for server side rendered templates.
@4nderss I took another look at it, the next suggestion for you would be to fix the table markup. The repeat shouldn't be on the <body> element, I think you meant to put that on a <tr>. The repeated <td> is missing a closing tag. The <th> elements should be within a <tr>. These fixes will speed things up but not much. Attempting to dynamically display 1000 rows * 17 cols in IE 11 is tough. IE's DOM is extremely slow. Practically every DOM manipulation operation is slower than chrome/firefox/etc by a factor of 4 or more. To get a sense of what I'm talking about, try out the DOM benchmarks in http://dromaeo.com/ (created by John Resig, author of JQuery).
@jdanyow I have added your proposed changes. The tbody is by choice since we have 2-3 rows per row template, and that is why I brought it along in my example. As you suggested the html error fixes did not improve overall performance.
What I'm trying to figure out is why the has() function is taking alot of time, and why it is called thousands of times. Just having the function run for 1ms adds another 1000ms to page speed if called 1000 times. You can run the performance tool in IE to get a better understanding.
As you can see in the picture above, the function is stealing alot of cpu time. I understand that IE DOM operations are slower, but if 25% of the render time is wasted inside one function (not including all the time wasted on steps before calling the actual function).
What I also find interesting is the 7.25 sek garbage collection. Seen in the picture above, don't know if that is normal.
Is there any way to swap out aurelia-polyfills for core-js or es6-shim in my example to try and test performance?
As a test, try putting core-js in a script tag in your index.html document's head. It will install it's polyfills, causing aurelia to skip installing it's own.
@jdanyow I switched to core-js shims by adding
<script src="node_modules/core-js/client/shim.min.js"></script>
inside my header and performance went up alot. I did a comparison and here are the results (3 pageloads per case).
Chrome/Win 10 (doesn't need polyfills) : 2307ms
IE11/Win 10 Aurelia Polyfills: 8434 ms avg
IE11/Win 10 core-Js: 4161 ms avg
IE11/Win 10 es6-shim: 14872 ms avg (although this looked very strange in profiler, so not 100% sure about this).
This means IE11 is 80% slower than Chrome with core-js and that is alot more acceptable than 365% that aurelia has.
I think there must be something strange going on inside aurelia polyfills. Atleast there should be room for improvement.
Also the profiler looks way better in core-js where the cpu time is used on a function called appendNodesTo.
I'm gonna try this in our app and get back with results
@4nderss - can you try one other thing while you're profiling:
Here's a new build of binding that removes Map usage from the connect queue. Curious if it makes any difference for you: dist.zip
If you're using jspm, overwrite your jspm_packages/npm/aurelia-binding@... folder with the contents of the dist/amd in the attached zip.
If you're installing aurelia with npm, use the dist folder to replace your node_modules/aurelia-binding/dist folder.
@jdanyow ok here are the results. Looks alot more promising now.
The has() does not show while profiling, result is now more like core-js.
Average render time with my test in IE11/Win 10: 4071ms, so faster than core-js it seems. 👍
Cool, sorry this got closed prematurely. I saw the heavy getter property and thought that was the issue.
No problemo, thanks for the fix. I prefer to use the use aurelia libs over importing core-js :)
@EisenbergEffect shouldn't the fix be released with aurelia-binding? Can't see any releases since Oct 6. Or by out, do you mean that it's ready but not yet released?
Ooops. Sorry. We missed the binding release. It's out now.
@EisenbergEffect, @jdanyow thanks, I have tested the update now. However it looks like something else with this update has affected our performance now.
IE went from 2500ms render with the above aurelia-binding fix from @jdanyow to 4000ms after npm updates. Tested with new aurelia release, with and without core-js loaded before aurelia.
From a quick overview it looks like it is doing alot of calls to subscribe on eventmanager from bind().
IE
I will try and reproduce in an isolated example..
For whetever reason this stopped happening today after a wípe of my npm folder. Performance is now better and we do not have to use core-js.
Thanks for fix and release. Keep up the good work!