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 issues in Angular SSR configuration #19

Open
cgrantnz opened this issue Sep 6, 2022 · 3 comments
Open

Performance issues in Angular SSR configuration #19

cgrantnz opened this issue Sep 6, 2022 · 3 comments

Comments

@cgrantnz
Copy link
Contributor

cgrantnz commented Sep 6, 2022

Hello,

I think that this is a great project, Angular Universal has lacked public performance benchmark data since it's so different to other frameworks and Universal has been difficult to set up until quite recently. I think there are a few issues affecting the performance of the Angular Universal example which is currently the slowest SSR example.

Things like:

If we make some PRs for that is there a contribution process or should we just make a bunch of PRs?

Thanks!

@steve8708
Copy link
Contributor

Thanks for this @cgrantnz! So we're absolutely game for PRs to improve. Funny enough from chatting with @mgechev I recently updated to remove inlineCriticalCss (I believe I did it right - can you confirm?)

I haven't updated the numbers since just yet though

We can also absolutely update to use trackBy and onPush, those would need to be updates to Mitosis as a key thing now is that we are constantly updating the example code, so as long as mitosis can reliably output to the desired format we're good there. @samijaber can help you regarding contributing there too

re: not calling methods from inside templates, in theory that could be done in Mitosiis too (or is helped by onPush), but seems a little trickier. our goal here is to not overly hand optimize code, and just see how each framework handles imperfect code, so if we are calling methods repeatedly in other frameworks it seems like we should do that for angular too

lmk if this makes sense though, and we can provide some guidance on the mitosis side if you are up to contribute there

@cgranttm
Copy link

cgranttm commented Sep 9, 2022

@steve8708 I profiled the SSR page loads and it seemed like it was spending lots of time in the CSS extraction code so I think it might need to be added to the SSR build config as well. I'll double check that.

The method calls inside templates shouldn't be too much of an impact I'll check the difference with that change. I think that that's a thing where angular and react/tsx based frameworks diverge. When a variable changes in a react template it doesn't scan the entire view tree to see if something else changed, whereas angular does https://angular.io/guide/change-detection-slow-computations (key part: "Change detection evaluates all template expressions in all components, unless specified otherwise, based on that each component's detection strategy"). When it can't check for equality cheaply it would have an impact. Anyway I think it should be fine, this would be tricky to model in Mitosis.

@cgrantnz
Copy link
Contributor Author

Simple benchmark with inline critical enabled in Angular (current state):
Running 20s test @ http://localhost:4200/dashboard
1 connections

┌─────────┬──────┬──────┬───────┬───────┬─────────┬────────┬───────┐
│ Stat │ 2.5% │ 50% │ 97.5% │ 99% │ Avg │ Stdev │ Max │
├─────────┼──────┼──────┼───────┼───────┼─────────┼────────┼───────┤
│ Latency │ 8 ms │ 8 ms │ 13 ms │ 16 ms │ 9.11 ms │ 2.5 ms │ 89 ms │
└─────────┴──────┴──────┴───────┴───────┴─────────┴────────┴───────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬─────────┬────────┬─────────┐
│ Stat │ 1% │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Req/Sec │ 64 │ 64 │ 108 │ 110 │ 105 │ 9.93 │ 64 │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Bytes/Sec │ 1.53 MB │ 1.53 MB │ 2.58 MB │ 2.63 MB │ 2.51 MB │ 237 kB │ 1.53 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴─────────┴────────┴─────────┘

Req/Bytes counts sampled once per second.

Inline critical disabled:

Running 20s test @ http://localhost:4200/dashboard
1 connections

┌─────────┬──────┬──────┬───────┬───────┬─────────┬─────────┬───────┐
│ Stat │ 2.5% │ 50% │ 97.5% │ 99% │ Avg │ Stdev │ Max │
├─────────┼──────┼──────┼───────┼───────┼─────────┼─────────┼───────┤
│ Latency │ 5 ms │ 5 ms │ 9 ms │ 10 ms │ 5.71 ms │ 1.68 ms │ 67 ms │
└─────────┴──────┴──────┴───────┴───────┴─────────┴─────────┴───────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬─────────┬────────┬─────────┐
│ Stat │ 1% │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Req/Sec │ 102 │ 102 │ 165 │ 173 │ 162.65 │ 15.1 │ 102 │
├───────────┼─────────┼─────────┼─────────┼─────────┼─────────┼────────┼─────────┤
│ Bytes/Sec │ 2.44 MB │ 2.44 MB │ 3.94 MB │ 4.13 MB │ 3.88 MB │ 360 kB │ 2.44 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴─────────┴────────┴─────────┘
This is a one line change so I'll make an MR for it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants