krausest / js-framework-benchmark

A comparison of the performance of a few popular javascript frameworks
https://krausest.github.io/js-framework-benchmark/
Apache License 2.0
6.58k stars 814 forks source link

Angular Signals, Control Flow and general improvements #1403

Closed birkskyum closed 8 months ago

birkskyum commented 8 months ago

Angular 16 introduce Signals. Angular 17 introduce Control flow - replace ngFor with @for

I've verified isKeyed on the control-flow version.

krausest commented 8 months ago

Thanks. From what I know angular 16 doesn't use signals for fine grained reactivity yet. So for v16 it seems like signals in angular are disappointing regarding their performance. I'd be open to add a preview of v17 with the new control flow syntax. Does anyone know if this new syntax already helps making rerendering for signals faster?

birkskyum commented 8 months ago

The new control flow @for makes the swap row way faster. I saw local results drop from 170 -> 25.

krausest commented 8 months ago

You might look it there's a NaN value in the same row for some other framework. If that's the case delete the offending file from the result directory.

birkskyum commented 8 months ago

Massive improvement for Angular

Screenshot 2023-10-13 at 15 12 04
krausest commented 8 months ago

I'd say the future is signals and zone-less. What's that version with 1,27? Signals and control-flow?

birkskyum commented 8 months ago

@krausest , it's actually zone.js and control-flow

only change to the angular suite is swapping ngFor with @for

Might be that the fastest combination is nozone+control-flow or signals+control-flow. I'll put it to the test now

There are 6 combinations in total

1 - 16: angular (zone.js + ngFor) 2 - 17: angular-control-flow

3 - 16: angular-nozone 4 - 17: angular-nozone-control-flow

5 - 16: angular-signals 6 - 17: angular-signals-control-flow

I'm happy to provide either - which would you like to have in the bench?

krausest commented 8 months ago

I think angular is big enough such that two combinations for v17-next would be fine (control-flow, zone.js, classic change detection and control-flow, signals, without zone.js)

birkskyum commented 8 months ago

Okay, so you'd like 1-5, but the 6th version angular-signals-control-flow should be left out. Did I get that right?

krausest commented 8 months ago

Ah ok, 6 is quite a number. Let me see:

2 and 6 are most important for the future of angular and 5 shows where's angular right now with signals. So I'd go with 2,5 and 6.

krausest commented 8 months ago

if 1 is the PR you've submitted here, then we can also add 1 as it doesn't add a new implementation.

birkskyum commented 8 months ago

This PR currently has optimized the existing 1, 3 and introduce 5 (signals)

birkskyum commented 8 months ago

I've made all 6, we can have a look at the bench in a few min, and then decide what to remove.

krausest commented 8 months ago

So then I'd say 1,2,3,5 and 6. Once v17 gets released I'd remove the v17-next implementations. Thanks!

birkskyum commented 8 months ago

The numbers are in from local testing. Just to double check - you'd like to exclude no. 4, the angular-nozone-control-flow, which is the fastest of the bunch?

Screenshot 2023-10-13 at 17 09 14
krausest commented 8 months ago

We can't leave out the fastest 😄 So finally you sold my all 6 👍

birkskyum commented 8 months ago

I've updated the PR to include all 6.

birkskyum commented 8 months ago

I rebased the whole thing

birkskyum commented 8 months ago

Svelte 4 for reference

Screenshot 2023-10-13 at 17 37 39
leeoniya commented 8 months ago

soo...this feels like a lot of extra noise / scrolling. im not sure people want to see a table that consists mostly of react and angular variants. im not saying we should remove them but some kind of collapsing/grouping would be a significant improvement.

maybe even dedicated tables for frameworks with more than two variants or multiple state management solutions?

maybe leave the slowest (or most idiomatic) and fastest angular variants in the main table with an extra table for all angular.

birkskyum commented 8 months ago

One solution would be to only keep the versions with new control flow when Angular 17 is out (scheduled for November), since it's pushed as the new standard with a trivial migration path.

leeoniya commented 8 months ago

i think it's important to keep the version(s) that are likely to still have a large usage share, as this encourages people to move to newer versions and better patterns, if they're faster. so latest & greatest + what's likely to be the user's current reality.

birkskyum commented 8 months ago

Good point. When Angular 17 is out, it wouldn't hurt to remove item 3 and 5 in this list, but keeping item 1 to show the difference between current ngFor and new @for could make sense.

pkozlowski-opensource commented 8 months ago

@birkskyum @krausest I wasn't aware of this PR and just opened https://github.com/krausest/js-framework-benchmark/pull/1438 to update to v17 and the new control flow.

Reading through this discussion I would suggest:

Most importantly we should update to the RC.0 since it has some additional improvements (especially around the update scenario).

In any case: I'm happy to help here and get official Angular results on the board (disclosure: I'm part of the Angular team and worked on the new control flow / list diffing algorithm).

birkskyum commented 8 months ago

Given how chrome 118 benchmark numbers were just released, and Angular 17 will be out soon (November?), I think it's best to wait of the official release, and then update this benchmark accordingly. As suggested above the control flow/zone.js should be the new 'angular' default, and then we could have three interventions

Will the slower ngFor etc. stay in angular 17? An if so, why?

pkozlowski-opensource commented 8 months ago

Given how chrome 118 benchmark numbers were just released, and Angular 17 will be out soon (November?), I think it's best to wait of the official release, and then update this benchmark accordingly.

@birkskyum we do have the final release planned before the 10th of November. Still I would love to see results on RC - it is pretty stable atm and it would allow us to use independently verified numbers in the final release communication.

As suggested above the control flow/zone.js should be the new 'angular' default, and then we could have three interventions

As I've mentioned I don't think including signals would be rather confusing at this point so I would suggest skipping this bit and go with:

Will the slower ngFor etc. stay in angular 17? An if so, why?

It will stay till we make sure that the new control flow doesn't introduce any subtle breaking changes and give people time to migrate.

pkozlowski-opensource commented 8 months ago

@birkskyum btw, if we want to show signals I would suggest shaping the data differently - as of today Angular don't have fine-grained data updates so wrapping each label in a signal is not helping (I think it actually slows down things). I will have a look at this scenario more but yeh, the impl proposed in this PR for signals could be faster - will review.

birkskyum commented 8 months ago

@pkozlowski-opensource thanks for the suggestion, I removed the signals from the label.

I updated everything to 17.0.0-rc.0, and organized the tests accordingly to consider control-flow is the new default (by removing ngfor-nozone and ngfor-signals). This makes the ngfor vs. cf an apples-to-apples comparison by testing with the same angular 17.0.0-rc.0 version.

@krausest A plan could be to include this in the Chrome 119 benchmark , so it'll be ready for the Angular announcement, and then update 17.0.0-rc.0 -> 17.0.0 when Angular 17 is out.

Screenshot 2023-10-19 at 19 35 07
pkozlowski-opensource commented 8 months ago

@birkskyum this sounds fantastic, thank you so much for doing all those updates!

krausest commented 8 months ago

I'm pretty sure I can merge it the next few days and include it in the preliminary results.

pkozlowski-opensource commented 8 months ago

@birkskyum also good move of dropping a signal for label - at this point it was just adding overhead without providing much benefit. I'm still a bit surprised that we see this amount of overhead, time to poking around with a profiler!

Again, thnx so much for doing all this!

krausest commented 8 months ago

Thanks! So here we go. I performed an incremental run (only changed implementation) that turned out as quite odd. This run used chrome 118.0.5993.88. It seems like most (if not all) frameworks got worse results than for the chrome 118 full run (vanillajs and wasm-bindgen we're unchanged and serve as a control group). Maybe some security fix causes the slow down. So a comparison between frameworks of that last run and of the official run are likely wrong. This is why the preliminary results carry the warning text :) Maybe I can perform a full run for the keyed table this weekend. Screenshot 2023-10-20 at 8 07 53 PM

HyperLife1119 commented 8 months ago

Is it possible to rename angular-cf to angular-@for so that it looks easier to understand.

leeoniya commented 7 months ago

cc @StephenFluin

birkskyum commented 7 months ago

@pkozlowski-opensource , it's part of the newly published 119 results.

pkozlowski-opensource commented 7 months ago

Thnx for the info @birkskyum - very much appreciate it, timing is just perfect!