BuilderIO / framework-benchmarks

Test each framework for it's performance cost
464 stars 24 forks source link

Performance issues in Angular SSR configuration #19

Open cgrantnz opened 1 year ago

cgrantnz commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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.