rrousselGit / freezed

Code generation for immutable classes that has a simple syntax/API without compromising on the features.
https://pub.dev/packages/freezed
1.91k stars 235 forks source link

Performance regression related to lists, `copyWith` and `==` in V2.0 #653

Closed SecondFlight closed 2 years ago

SecondFlight commented 2 years ago

Hi, thanks for the library! I have greatly appreciated using it.

After updating freezed to 2.0, my Flutter app has seen a significant performance regression that worsens over time.

I have not been able to find this reported anywhere else, though I apologize if I've missed something. I believe this differs from #626 as that report is talking in general about the performance of DeepCollectionEquality and doesn't mention deteriorating performance over time.

Reproduction

I believe I've isolated a minimal reproduction case. Here's a code sample:

myModel.dart:

import 'package:freezed_annotation/freezed_annotation.dart';

part 'my_model.freezed.dart';

@freezed
class ModelWithList with _$ModelWithList {
  factory ModelWithList({
    @Default([]) List<int> someList,
    @Default(0) int counter,
  }) = _ModelWithList;
}

main.dart:

import 'my_model.dart';

void main(List<String> arguments) {
  var model = ModelWithList(someList: List.filled(1000000, 0));

  final stopwatch = Stopwatch()..start();

  var elapsed = 0;

  for (var i = 0; i < 10000; i++) {
    final oldModel = model;
    model = model.copyWith(counter: model.counter + 1);

    if (oldModel == model) {
      throw Exception("New model should not equal old model.");
    }

    final elapsedThisIteration = stopwatch.elapsedMicroseconds;

    if (i % 10 == 0) {
      print("Delta: ${(elapsedThisIteration - elapsed) / 1000000} seconds");
    }

    elapsed = elapsedThisIteration;
  }
}

Expected behavior

I expect the print statement to display a relatively constant time throughout the duration of the loop, no matter how slow or fast.

Actual behavior

When using freezed: ^1.1.0, I get an output like this:

Delta: 0.000178 seconds
Delta: 0.000236 seconds
Delta: 0.000192 seconds
Delta: 0.000197 seconds
Delta: 0.000221 seconds
Delta: 0.000182 seconds
...

The average duration does not appear to change significantly over time.

When running with freezed: ^2.0.2, I get an output like this:

Delta: 0.034594 seconds
Delta: 0.117922 seconds
Delta: 0.319261 seconds
Delta: 0.423841 seconds
Delta: 0.520171 seconds
Delta: 0.635811 seconds
Delta: 0.791081 seconds
...

The delta continues to increase over time. Most of the time here seems to be spent in the oldModel == model equality check. Commenting it out results in this output (using ^2.0.2):

Delta: 0.000236 seconds
Delta: 0.000345 seconds
Delta: 0.000275 seconds
Delta: 0.000284 seconds
Delta: 0.000229 seconds
Delta: 0.000222 seconds
...
rrousselGit commented 2 years ago

Hello!

Tha ms for the detailed report. My immediate guess is that it could be related to immutable collections.

I'm not sure if I'll be able to check that today, so try disabling it and see if the issue disappears 😊

SecondFlight commented 2 years ago

Thanks for the quick reply and the suggestion! Adding makeCollectionsUnmodifiable: false to the @Freezed annotation does seem to fix the issue, so I'm using it as a workaround for now.

// Workaround for https://github.com/rrousselGit/freezed/issues/653
@Freezed(makeCollectionsUnmodifiable: false)
class MyModel with _$MyModel {
  // ...
rrousselGit commented 2 years ago

Thinking about this issue, I think it may be worth disabling this feature in release mode

As in, before making a release build, do:

dart run build_runner build --release

which would generate the Freezed files again, but with makeCollectionsUnmodifiable disabled.

rrousselGit commented 2 years ago

For reference, you can do this --release thing today:

targets:
  $default:
    builders:
      freezed:
        release_options:
          make_collections_unmodifiable: false
hacker1024 commented 2 years ago

What about in packages? I maintain a client package for a HTTP API with a lot of Freezed models exposed in the package API.

Ideally, I'd like collections to be unmodifiable to users of the package. There's no easy way for them to rebuild the package files in release mode, though.

knaeckeKami commented 1 year ago

To add some more context to this, it might really be a worthwhile optimization to try to avoid DeepCollectionEquality when possible.

It is ~5x slower than ListEquality() in simple cases, and has quadratic complexity when the collection is actually nested ( O(n^2) runtime, where n is the level of nesting in the collection).

So, a JSON Map<String, dynamic> with nesting level of 3 in a freezed object already takes ~60x longer to compare using == than an optimized version.

See this tracking issue: https://github.com/dart-lang/collection/issues/263

and benchmark that I wrote: https://github.com/knaeckeKami/json_equals

And, as Jake Wharton puts it,

A code generator is only written once but the code it generates occurs many times. Thus, any investment into making the generator emit more efficient code will pay for itself very quickly. This generally means output less code and allocate fewer objects wherever possible. I’d like to expand on that with two specific, real-world examples which I’ve run into.

in https://jakewharton.com/the-economics-of-generated-code/

rrousselGit commented 1 year ago

Yes, there is an issue about it.

I just don't care too much about it, as Freezed will have to be rewritten soon with metaprogramming.

knaeckeKami commented 1 year ago

sorry, yeah I did not mean to comment on this closed issue, i copied it in the open issue.

I just don't care too much about it, as Freezed will have to be rewritten soon with metaprogramming.

Fair enough, I just wanted to bring some attention to it that someone might run into this (I ran into this in a app of mine and was not aware of the quadratic complexity of deepcollectionequality so I took me some time to figure out was was going on)