Skip to content

Surprisingly poor performance of as casts in Flutter Framework with dart2js #48315

Open
@jonahwilliams

Description

@jonahwilliams

See also: flutter/flutter#97822 / flutter/flutter#97249 / https://docs.google.com/document/d/1jkI8GCT4cId3a87PV5E0XQWRd2O5HsNhmchwgDUDv_k/edit

Flutter's Element class exposes a getter Widget get widget => ... which subclasses will refine to a more specific type using a cast. While this is technically unsound, by construction it is mostly safe. It looks roughly like:

class Element {
  Widget get widget => ...
}


class StatelessElement extends Element {
  StatelessWidget get widget => super.widget as StatelessWidget;
}

I noticed in chrome traces that the as cast for this widget getter was showing up in the top% of CPU while interacting with flutter applications, so I investigated splitting this API up so that most callsites would use Element.widget without casting:

class Element {
  Widget get widget => ...
}


class StatelessElement extends Element {
  StatelessWidget get typedWidget => super.widget as StatelessWidget;
}

The result was an easily measurable performance improvement in first builds. Because there are many places where Element.widget is used without any regard to the specific subtype the change is fairly self contained, except for the fact that it forces us to duplicate our API here in an odd way.

The benchmark time decreased from 1845 ms to 1688 ms with the change applied from 97822

// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:ui' as ui show window;
import 'package:flutter/material.dart';

void main() async {
  final binding = WidgetsFlutterBinding.ensureInitialized();
  ui.window.onPointerDataPacket = null;

  var root = MediaQuery.fromWindow(
      child: Directionality(
    textDirection: TextDirection.ltr,
    child: Material(
      child: Column(
        children: List<Widget>.generate(9, (index) {
          return ListTile(
              leading: CircleAvatar(child: Text(index.toString())),
              title: Text('Content $index'));
        }),
      ),
    ),
  ));
  var results = [];
  for (var i = 0; i < 10; i++) {
    var sw = Stopwatch()..start();
    for (var count = 0; count < 1000; count++) {
      var renderViewElement = RenderObjectToWidgetAdapter(container: binding.renderView, child: root)
        .attachToRenderTree(binding.buildOwner!, null);
    }
    sw.stop();
    results.add(sw.elapsedMilliseconds);
    await null;
  }

  print(results);
}

I also investigated other approaches:

  • We could use the unsafeCast functionality documented in Need a way to avoid some type checks without implicit downcasts #31428, but that problem is that there are many Element subclasses, some of which aren't in the framework. I'd be worried about the pattern being cargo culted, used in unsafe ways, et cetera. It also does not have a non-zero cost in gen_snapshot.

  • We could make Element generic. This is a manageable breaking change, but it then prevents us from making the Element._widget backing field non-nullable.

Ideally, an as cast would not have such an oversized impact on the framework performance. Not to mention there are likely more places throughout the framework and engine that would benefit from improved as cast performance in ways that are harder to measure

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-web-jsIssues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop.web-dart2js

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions