-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
IE11 polyfill performance #39
Comments
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 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: |
This free performance advice brought to you in part by @jdanyow :) |
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 |
@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 Chrome/Win 10 (doesn't need polyfills) : 2307ms This means IE11 is 80% slower than Chrome with core-js and that is alot more acceptable than 365% that aurelia has. 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 If you're using jspm, overwrite your If you're installing aurelia with npm, use the dist folder to replace your |
@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 :) |
@jdanyow when is this fix scheduled for release? |
It's out. Release announcement is coming this week but the code is out.
…On Dec 6, 2016 11:41 PM, "Anders Ulfheden" ***@***.***> wrote:
@jdanyow <https://github.com/jdanyow> when is this fix scheduled for
release?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIBnVYpBai9ILe45T9eRcBzPvEjP2aTks5rFmMagaJpZM4Kvhx1>
.
|
@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(). 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! |
I'm submitting a bug report
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?
The text was updated successfully, but these errors were encountered: