passsy / dart-lint

An opinionated, community-driven set of lint rules for Dart and Flutter projects. Like pedantic but stricter
Apache License 2.0
277 stars 82 forks source link

Suggestion: const can be left out in more ways than the lints allow #22

Closed nstrelow closed 3 years ago

nstrelow commented 3 years ago

Nice linter! Really appreciate the explanation for every rule choice.

According to some resources, const is in some cases optional. Meaning it gets added by the runtime and you do not gain any performance benefits by adding const. But I am not 100% sure, as in some cases it ensures that const is definitely used and thus has better performance.

IMO I would leave const out, trust the DartVM to optimize most of it and for that VERY expensive constructor I would leave it to the developer to add it in.

This should clean up code in flutter a lot, since a lot of widgets can be const.

Some resources:

const is optional inside of a constant context.

Dart2 Migration guide: https://dart.dev/dart-2

In short, even without the keyword, we can still unambiguously recognize the expressions that create objects. In that sense, the keywords are superfluous.

Similarly, it may be important for developers to ensure that certain expressions are constant, because of the improved performance and the guaranteed canonicalization.

https://github.com/dart-lang/sdk/blob/master/docs/language/informal/implicit-creation.md

Here are some very good examples on where it is optional:

https://stackoverflow.com/questions/52581148/when-is-const-optional-in-dart-2

nstrelow commented 3 years ago

Lints that can be disabled by my understanding (and I am not 100% sure):

The rest of the const constructors do not pollute the code so much. I am not sure if they are needed there everywhere, but there are fewer cases of them (at least for my flutter code).

passsy commented 3 years ago

There are two relevant rules here:

Together you'll only see const warnings where you could add const but the compile will not automatically add it.

Magic const doesn't exist

const is automatically added by the Dart VM

Your claim is wrong. const is only automatically added in const contexts. But in those cases, you will never see a warning to add const. The lint check is clever enough.

The implicit-creation proposal was changed in 2018 and removed the "magic const" part (I also had it in mind differently for a long time).

0.6 (2018-04-06) Removed "magic const" again, due to the risks associated with this feature (getting it specified and implemented robustly, in time).

It now works like this

We define new/const insertion as the following transformation, which will be applied to specific parts of the program as specified below:

  • if the expression e occurs in a constant context, replace e by const e,
  • otherwise replace e by new e.

In a codebase without a single const a const context is never created, and the compile can't create const automatically. Therefore you should add const whenever possible and this lint rule does exactly that.

Why can't the compiler not automatically add const?

If the compiler would add const automatically it would change the behaviour of some code. identical checks would behave differently as shown in the stackoverflow questions:

identical(const Object(), const Object()); //true
identical(Object(), Object()); //false
identical(new Object(), new Object()); //false

Automatically inserting const outside a const context could also lead to different behaviour between AOT and VM code.

Also check out the dart language tour about const constructors.

If a constant constructor is outside of a constant context and is invoked without const, it creates a non-constant object:

var a = const ImmutablePoint(1, 1); // Creates a constant
var b = ImmutablePoint(1, 1); // Does NOT create a constant

assert(!identical(a, b)); // NOT the same instance!
nstrelow commented 3 years ago

Thanks a lot for the explanatory comment ;D

@passsy Do you think it makes a difference performance wise?

https://stackoverflow.com/a/62825321/1387765

For my results there is no difference in frame per second and there is no difference in memory usage except that the Garbage collector seems to run more often when not using const. Again, the FPS were about the same.

If that source is trustworthy

passsy commented 3 years ago

Yes, it really makes a difference. Flutter checks for equality when widgets change. When newWidget == oldWidget it doesn't call didUpdateWidget(oldWidget). This equality check is only true when both widgets are identical which is true for const widgets.

const Center() == const Center(); // true
Center() == Center(); // false

framework.dart - Element#updateChild

  @protected
  Element? updateChild(Element? child, Widget? newWidget, dynamic newSlot) {
    if (newWidget == null) {
      if (child != null)
        deactivateChild(child);
      return null;
    }
    final Element newChild;
    if (child != null) {
      bool hasSameSuperclass = true;

      if (hasSameSuperclass && child.widget == newWidget) { // <-- this is true for const Widgets
        if (child.slot != newSlot)
          updateSlotForChild(child, newSlot);
        newChild = child;
      } else if (hasSameSuperclass && Widget.canUpdate(child.widget, newWidget)) {
        if (child.slot != newSlot)
          updateSlotForChild(child, newSlot);

        child.update(newWidget); // <-- skipped didUpdateWidget call

        newChild = child;
      } else {
        deactivateChild(child);
        newChild = inflateWidget(newWidget, newSlot);
      }
    } else {
      newChild = inflateWidget(newWidget, newSlot);
    }
    // ...
nstrelow commented 3 years ago

Thanks, so such a Widget is not rebuilt.

I guess most default flutter widgets, such as Container, are cheap to built.

But a expensive Widget getting rebuilt everytime would be a drastic performance impact.

Thanks for further explanation. I created an issue in the flutter repo in hopes of providing a clear official Flutter recommendation.

I believe the Dart docs somehow explain it in the const constructor part.

passsy commented 3 years ago

If you have a large tree of const Widgets, this part never gets rebuild and no constructor is invoked. const objects are already in memory with a fixed pointer.

// everything is const, the whole tree causes 0 allocations 
// no matter how often it gets called
const Center( 
  child: SizedBox(
    height: 35,
    width: 43,
    child: Padding(
      padding: EdgeInsets.all(right: 8),
      child: Icon(Icons.eleven_mp),
    ),
  ),
);

Without const, this code would cause 5 allocations every time it is executed.

Again, that alone won't make you skip a frame but it adds up. Rebuilding an app may cause 1000s of Widget constructor calls. Making as much as possible of them const will help.