Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong information in readme #4

Closed
rrousselGit opened this issue Sep 21, 2018 · 4 comments
Closed

Wrong information in readme #4

rrousselGit opened this issue Sep 21, 2018 · 4 comments

Comments

@rrousselGit
Copy link

setState() is called on the root widget and it means that every widget in the tree should be rebuilt. Flutter is smart enough to make just incremental changes, but in general this is not so good.

This isn't true.
First of all, a setState on the root widget doesn't rebuild the whole widget tree.
Secondly, flutter doesn't do any sort of incremental change.

In flutter things are based around the class instance. If you reuse the same class instance as the old build call, flutter will stop any further build.

Which is why the following:

class _State extends State<Foo> {
  @override
  Widget build(BuildContext context) {
    Future.microtask( () => setState(() {}) );

    return widget.child
  }
}

will actually never force its child to rebuild even if it updates every frames.

@p69
Copy link
Owner

p69 commented Sep 24, 2018

Hi @rrousselGit , first of all thank you for opening this interesting issue, because the statement in readme isn't accurate.

Yes, you're right, that's calling setSet from a parent-widget doesn't necessary force to rebuild its child. But I was saying about Dartea application, not about Flutter in general. Typical Dartea application widgets tree looks like this:

                        MaterialApp (or Cupertino, or WidgetsApp)
                                               |
                                          DarteaWidget
                                               |
                                         Some container
                                        /         |        \  
                                  Widget1      Widget2   Widget3

In this hierarchy only DarteaWidget is stateful and its setState() is called when model has changed. It triggers rebuild for DarteaWidget and inside it view function is called, which is usually returns new instance of some widget, Dartea doesn't do any caching or memoization. In current case every model change triggers rebuild for the whole widgets tree (which is defined by composition of view functions). And this is fine for Elm application, and I guess that is not so bad for Flutter application.

By incremental changes I meant that rebuilding widgets tree doesn't force to recreate all render objects, instead it just do incremental updates properties of render objects. I just wanted to say, that creating of Widgets is cheap in Flutter.

Please correct me if I'm wrong, or if it's still unclear to you.
I want to update this section in readme + maybe give some advice for performance improvements in Dartea applications.

@rrousselGit
Copy link
Author

I see, I misunderstood that part then.

And this is fine for Elm application, and I guess that is not so bad for Flutter application.
By incremental changes I meant that rebuilding widgets tree doesn't force to recreate all render objects, instead it just do incremental updates properties of render objects. I just wanted to say, that creating of Widgets is cheap in Flutter.

One difference with Elm is that the browser only redraw what changed. In flutter this is not the case. If a single RenderObject update, the whole tree may repaint with it unless you add repaintboundaries.
But with such method, you can't even use repaintboundaries as the widget tree is always entirely recreated.

This may be fine for relatively small layouts. But I don't quite see it efficient for huge tree with thousands of widgets in it.

Exposing the Model using an InheritedModelWidget would be much more efficient.

@p69
Copy link
Owner

p69 commented Sep 24, 2018

I see what you mean.

I believe that there should be performance degradation on a huge widgets tree, but still I want to try profiling and investigation first. I've tried debugRepaintRainbowEnabled property to visualize repainted boxes, and it looks like not whole tree is repainted, see attached gif.

Also I can try to add possibility to memoize results of view function somehow (something similar to current Redux approach). Even though currently we can do it manually, but I think it should be out of the box.

todo_mvu_repainting

@rrousselGit
Copy link
Author

After investigating a bit, it does seems like if no properties of any RenderObject change, then there's no paint step requested. And even if the widget itself change, it doesn't necessarily end with a layout step.

So in the end, you are right, it does seem like if no RenderObject changes at all, "nothing" happens. It will still rebuild the whole tree, but not necessarily relayout and repaint everything.

Anyway, I'd still suggest investigating the performance impact of rebuilding a tree of 600+ widgets.
In the current state of the README it's hard not to fear performances problems on big applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants