Optimize copyWith code size

[rrousselGit]

The generated code for copyWith can be reduced in some situations.

For example if there is no need for "deep copy", some classes can be removed.

It's also possible to remove the interface for the redirecting class, which currently is only useful for copyWith, by doing:

class _Person {
  MyClass get copyWith({String name, int age}) {
    return ({Object? name = _default, Object? age = _default}) {
      return Person(...);
    };
  }
}

This could remove hundreds of lines of generated code

[fzyzcjy]

By the way the code sample you mentioned does not seem to work, maybe it is:

typedef PersonCopyWithFunctionSignature = _Person Function({String name, int age});

class _Person {
  PersonCopyWithFunctionSignature get copyWith {
    return ({Object? name = _default, Object? age = _default}) {
      return Person(...);
    };
  }
}

[fzyzcjy]

Btw I benchmarked it as well.

Not sure what is the _then so I ignore it when writing the modifiedCopyWith.

void main(List<String> arguments) {
  var dummy = 0;

  syncBenchmark('naive copyWith', () {
    var a = NaiveApple(id: 0, name: 'hello');
    for (var i = 0; i < 10000000; ++i) {
      a = a.copyWith(id: a.id + i, name: a.name);
    }
    dummy ^= a.id;
  }).report();
  syncBenchmark('freezed copyWith', () {
    var a = FreezedApple(id: 0, name: 'hello');
    for (var i = 0; i < 10000000; ++i) {
      a = a.copyWith(id: a.id + i, name: a.name);
    }
    dummy ^= a.id;
  }).report();
  syncBenchmark('freezed modified copyWith', () {
    var a = FreezedApple(id: 0, name: 'hello');
    for (var i = 0; i < 10000000; ++i) {
      a = a.modifiedCopyWith(id: a.id + i, name: a.name);
    }
    dummy ^= a.id;
  }).report();

}

@immutable
class NaiveApple {
  final int id;
  final String name;

  const NaiveApple({
    required this.id,
    required this.name,
  });

  NaiveApple copyWith({
    int? id,
    String? name,
  }) =>
      NaiveApple(
        id: id ?? this.id,
        name: name ?? this.name,
      );
}

@freezed
class FreezedApple with _$FreezedApple {
  const factory FreezedApple({
    required int id,
    required String name,
  }) = _FreezedApple;
}

extension ExtFreezedApple on FreezedApple {
  FreezedApple Function({int? id, String? name}) get modifiedCopyWith {
    return ({
      Object? id = freezed,
      Object? name = freezed,
    }) =>
        FreezedApple(
          id: id == freezed ? this.id : id as int,
          name: name == freezed ? this.name : name as String,
        );
  }
}

speed:

naive copyWith
          total runs:        377   
          total time:     2.0044  s
         average run:     5.3160 ms
         runs/second:     188.11   

freezed copyWith
          total runs:          7   
          total time:     2.2211  s
         average run:     317.30 ms
         runs/second:     3.1516   

freezed modified copyWith
          total runs:         17   
          total time:     2.0542  s
         average run:     120.83 ms
         runs/second:     8.2757

conclusion: 2.5x faster, but still 20x slower than naive solution...

[fzyzcjy]

Link: https://github.com/fzyzcjy/freezed_benchmark/commit/7f36b8a801a0a6385231e3c3a3b867fedf4cc940

[rrousselGit]

This issue isn't about performance. It's about size of the b generated code

[fzyzcjy]

I see. That sounds pretty valid. Btw also hope the performance could be better!

[rrousselGit]

Do note that your naive implementation is not at feature parity.

It's faster because it doesn't support null values

A case could be made that Freezed can avoid using the "== freezed" pattern for non-nullable values. That could improve performances I think.

[rrousselGit]

In particular, your benchmark allows doing "copyWith(nonNullable: null)".

This doesn't compile in Freezed as the copyWith parameter is correctly non-nullable in this case.

[fzyzcjy]

You are right. Looking forward to your optimizations!

[fzyzcjy]

By the way I come up with another grammar:

@immutable
abstract class ProposedApple {
  final int id;
  final String? name;

  const ProposedApple._({required this.id, required this.name});

  factory ProposedApple({required int id, required String? name}) => ProposedAppleImpl(id: id, name: name);

  ProposedApple copyWith({
    required int id,
    String? name,
  });
}

class ProposedAppleImpl extends ProposedApple {
  ProposedAppleImpl({required super.id, required super.name}) : super._();

  @override
  ProposedApple copyWith({Object? id = freezed, Object? name = freezed}) => ProposedAppleImpl(
        id: id == freezed ? this.id : (id as int),
        name: name == freezed ? this.name : (name as String?),
      );
}

  syncBenchmark('proposed copyWith', () {
    var a = ProposedApple(id: 0, name: 'hello');
    for (var i = 0; i < 10000000; ++i) {
      a = a.copyWith(id: a.id + i, name: a.name);
    }
    dummy ^= a.id;
  }).report();

It has correct null handling. But it is as slow as your proposed method. So just put it here FYI :)

[rrousselGit]

Making non-nullable required on copyWith doesn't make sense. Folks don't always want to change them

[fzyzcjy]

Oh you are right.

  ProposedApple copyWith({
    int id = 42, // any valid value is ok, since if user does *not* provide this arg, it indeed uses the overridden method's default arg (the "freezed") instead of the "42".
    String? name,
  });