Mobx.dart: ListView scroll jank if list row is trivial Observable

[stuz5000]

I'd like to populate a long ListView with interactive (stateful) elements.
Unfortunately, when I make a list item trivially depend on an mbox observable value, I get some terrible jank - when scrolling the list, the view stalls for around (my best guess) 1/4-1/2 seconds when scrolling - which goes away completely if the Observer widget wrapper is removed. It seems to be a much bigger issues for web than for mobile builds.

Am I doing something wrong?

import 'package:flutter/material.dart';
import 'package:flutter_mobx/flutter_mobx.dart';
import 'package:mobx/mobx.dart';

part 'demo_mobx.g.dart';

class MyState = _MyState with _$MyState;

abstract class _MyState with Store {
  @observable
  int value = 123;
}

MyState appState = MyState();

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  MyApp({Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
        debugShowCheckedModeBanner: false,
        title: 'MyApp',
        home: Container(
            color: Colors.white,
            child: ListView.builder(itemCount: 500,
                itemBuilder: listitem,    // <<<   > 1/4 second jank when scrolling
//                itemBuilder: rowContent,  // <<< Very fast
            )));
  }
}

final textStyle = TextStyle(
    color: Colors.black,
    decoration: TextDecoration.none,
    fontSize: 12,
    fontStyle: FontStyle.normal);

Widget listitem(BuildContext context, int i) => Observer(
    builder: (BuildContext context) => rowContent(context, i, appState.value));


Widget rowContent(BuildContext context, int i, [int j=0]) => Material(child:Text(
        "$i:  $j: abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef",
        style: textStyle));

I'm using:

  mobx: ^0.3.9
  flutter_mobx: #^0.3.3

and...

$ flutter doctor -v
[✓] Flutter (Channel unknown, v1.13.0, on Linux, locale en_US.UTF-8)
    • Flutter version 1.13.0 at /home/sir/development/flutter
    • Framework revision 09126abb22 (3 weeks ago), 2019-12-03 17:43:00 -0800
    • Engine revision 6179380243
    • Dart version 2.7.0 (build 2.7.0-dev.2.1 a4d799c402)


[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
    • Android SDK at /home/sir/Android/Sdk
    • Android NDK location not configured (optional; useful for native profiling support)
    • Platform android-29, build-tools 29.0.2
    • ANDROID_HOME = /home/sir/Android/Sdk
    • Java binary at: /home/sir/soft/android-studio/jre/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Android Studio (version 3.5)
    • Android Studio at /home/sir/soft/android-studio
    • Flutter plugin version 42.1.1
    • Dart plugin version 191.8593
    • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405)

[✓] IntelliJ IDEA Ultimate Edition (version 2019.3)
    • IntelliJ at /snap/intellij-idea-ultimate/192
    • Flutter plugin version 42.1.4
    • Dart plugin version 193.5731

[✓] Connected device (3 available)
    • Android SDK built for x86 • emulator-5554 • android-x86    • Android 10 (API 29) (emulator)
    • Chrome                    • chrome        • web-javascript • Google Chrome 76.0.3809.132
    • Web Server                • web-server    • web-javascript • Flutter Tools

• No issues found!

[pavanpodila]

I am not sure if virtualization is implemented for the Web version of ListView. That could be one reason why you are seeing such bad performance. Need to confirm this with the Flutter team.

[stuz5000]

Is virtualization needed by Observer?

To be clear, the list view is very performant for web without observable
(and I can have complex list rows consisting of rows, columns, cards, text,
canvasses, dragmanagers, icons). Just the addition of Observer make the
difference.

On Tue, Dec 24, 2019 at 12:27 AM Pavan Podila [e-mail]
wrote:

I am not sure if virtualization is implemented for the Web version of
ListView. That could be one reason why you are seeing such bad
performance. Need to confirm this with the Flutter team.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx.dart/issues/340?email_source=notifications&email_token=AB3QJLHHUBQVTB76WT6IDKTQ2HBVFA5CNFSM4J63KLCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHSY2TQ#issuecomment-568692046,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB3QJLBBUENUHQO5L7GTJNLQ2HBVFANCNFSM4J63KLCA
.

[pavanpodila]

That is good to know. So it might be some issue with a zombie Observer lingering even after an item scrolls past the viewport. Can you do a quick profile to find out if there is a memory leak or do a CPU profile to see where time is being spent ?

[stuz5000]

Almost all of the time is taken in mount>createReaction.

I performed some profiling on the initial rendering (code below)

• 85 elements in 0.308s. -- without Observer
• 85 elements in 6.383s -- with Observer

Via the chrome's profiler, it looks like most of the time is spend in

handleDrawFrame   (called once)
   >layout 
     > childAndLayout 
         >  sliver_multibox_adaptor.RenderSliverMultiBox.Adaptor  (x85 times)
             >  ...
               > mount
                 > createReaction 
                   > getName 
                     > toString  
                        > anonymous 
                          > (map lookup?)

Each of the sliver_multibox_adaptor.RenderSliverMultiBox.Adaptor calls take ~90ms... which seems to add up the number of elements rendered (one call only per rendered element).

Without the Observer, the per-row sliver_multibox_adaptor.RenderSliverMultiBox.Adaptor call goes from 90 to 2 ms.

import 'package:flutter/material.dart';
import 'package:flutter_mobx/flutter_mobx.dart';
import 'package:mobx/mobx.dart';

part 'demo_mobx.g.dart';

class MyState = _MyState with _$MyState;

abstract class _MyState with Store {
  @observable
  int value = 123;
}

MyState appState = MyState();

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  MyApp({Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) => MaterialApp(
      debugShowCheckedModeBanner: false,
      title: 'MyApp',
      home: Scaffold(
          body: ListView.builder(
        itemCount: 500,
        itemBuilder: listitem, // <<<   > 1/4 second jank when scrolling
//        itemBuilder: rowContent, // <<< Very fast
      )));
}

final textStyle = TextStyle(
    color: Colors.black,
    decoration: TextDecoration.none,
    fontSize: 12,
    fontStyle: FontStyle.normal);

Widget listitem(BuildContext context, int i) => Observer(
    builder: (BuildContext context) => rowContent(context, i, appState.value));

Stopwatch _StopWatch() {
  final time = Stopwatch();
  time.start();
  return time;
}

final Stopwatch time = _StopWatch();

Widget rowContent(BuildContext context, int i, [int j = 0]) {
  print(
      "$i ${(time.elapsedMilliseconds.toDouble() / 1000).toStringAsFixed(3)}s");

  return Text(
      "$i:  $j: abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef abcdef",
      style: textStyle);
}

[stuz5000]

Am guessing its the call to StackTrace.current.toString()?

Should debugAddStrackTraceInObserverName be true even when flutter runs as non-debug?

mixin ObserverWidgetMixin on Widget {
  /// An identifiable name that can be overriden for debugging.
  ///
  /// Defaults to `widget.toString()`, and if in debug mode, a part of the stacktrace is also added.
  // methods instead of getters so that classes that mix-in `ObserverWidgetMixin` can have const constructors.
  String getName() {
    String name;

    assert(() {
      if (debugAddStrackTraceInObserverName) {   <<<<<
        name = '$this\n${StackTrace.current.toString().split('\n')[3]}';
      }
      return true;
    }());

    // this will be applicable for release builds where there are no asserts
    return name ?? '$this';
  }

Possible'$this' is the problem.... but it seems its trivial when stepping into in the debugger.

I can't easily unset observer.debugAddStrackTraceInObserverName. Its not exported.

[pavanpodila]

Thanks for that awesome analysis @stuz5000. You are right, it's likely the stack trace generation that's causing the slow down. We forgot to expose it 🙁. Let me put that behind a member field and turn it on only for debug builds. Will get to it today.

Or if you feel warmed up for a PR, I'm happy to assist 🤞.

[pavanpodila]

The StackTrace is used only during debug and should be disabled in release mode. I have published a new version which exposes this field. Please take a look if the issue persists even after turning it off during debug mode.

[pavanpodila]

@stuz5000 any update on the Jank issue with the newly exposed field ?

[stuz5000]

thanks!
Will check - do I need to checkout the mobs source, or is there something I
can put in my pubspec to test?

On Thu, Dec 26, 2019 at 10:00 AM Pavan Podila [e-mail]
wrote:

@stuz5000 https://github.com/stuz5000 any update on the Jank issue with
the newly exposed field ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx.dart/issues/340?email_source=notifications&email_token=AB3QJLCKSJUTU7SR3B66MPDQ2TWKZA5CNFSM4J63KLCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHV5I5Q#issuecomment-569103478,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB3QJLBNU3GTLM7R5QFR3ITQ2TWKZANCNFSM4J63KLCA
.

[stuz5000]

I don't seem to be able to access it:
(have run flutter clean ; flutter pub upgrade

pubspec.yaml

dependencies:
  mobx: ^0.3.9
  flutter_mobx: ^0.3.4+2
...

import 'package:flutter_mobx/flutter_mobx.dart';
import 'package:mobx/mobx.dart';

  ...  {
  debugAddStackTraceInObserverName = false;
}
````



md5-5ca48cba07205f15b86d04d456d30869




I still get:

flutter_mobx:file:///home/sir/development/flutter/.pub-cache/hosted/pub.dartlang.org/flutter_mobx-0.3.4+2/lib/
mobx:file:///home/sir/development/flutter/.pub-cache/hosted/pub.dartlang.org/mobx-0.3.10/lib/
mobx_codegen:file:///home/sir/development/flutter/.pub-cache/hosted/pub.dartlang.org/mobx_codegen-0.3.10+1/lib/```

[stuz5000]

Would it be better to make this false by default?

[shyndman]

On most platforms the stack trace isn't noticeably expensive to calculate, so imo we should lean towards improving the developer experience where possible.

Maybe setting the flag's default to false if we're on the web?

Let me take a look at the versioning issue.

[pavanpodila]

For some reason the variable debugAddStackTraceInObserverName is not getting exported even though its public. Not sure why that is happening. @shyndman any ideas ?

[shyndman]

I can see it, no problem. Here's the entry from my pubspec.lock

  flutter_mobx:
    dependency: "direct main"
    description:
      name: flutter_mobx
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.4+2"

[shyndman]

Any word @stuz5000?

[stuz5000]

Sorry for slow response. My local package agrees with the mofications in the PR https://github.com/mobxjs/mobx.dart/commit/d3635518c258b23142d293382b02640a617a6d85

... yet I can't see the debugAddStrackTraceInObserverName in the exported variables.

Seems I'm missing something about how to import that name, or the name was not exported.

import 'package:flutter_mobx/flutter_mobx.dart' as fm;

void f() {
  // Can see StatelessObserverElement in exports
  var l = List<fm.StatelessObserverElement>();   //OK

  // Cannot see StatelessObserverElement in exports
  var x = fm.debugAddStrackTraceInObserverName;   // Fails
}

... gets the compile error.

error: The name 'debugAddStrackTraceInObserverName' is being referenced through the prefix 'fm', but it isn't defined in any of the libraries imported using that prefix. (undefined_prefixed_name at [flutterapp] lib/mobxexample.dart:16)

My pubspec.yaml says:

dependencies:
  mobx: ^0.3.9  # 0.3.10 installed
  flutter_mobx: ^0.3.4+2 

[shyndman]

Just to be completely clear, the definition of debugAddStackTraceInObserverName is present in observer.dart? But the analyzer is failing to see it?

If so, you might want to try delete the analyzer's cache, ~/.dartServer.

[shyndman]

What's the word @stuz5000?

[stuz5000]

@shyndman Yes. The variable is in observer.dart. Deleting dartServer and compiling from the command line still gets the error.

[shyndman]

Ah, I see the problem. There's a typo in your source.

Should be debugAddStackTraceInObserverName.

[pavanpodila]

replace debugAddStrackTraceInObserverName ==> debugAddStackTraceInObserverName

[stuz5000]

Good grief