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

Review flutter attach performance #53313

Open
xster opened this issue Mar 25, 2020 · 9 comments
Open

Review flutter attach performance #53313

xster opened this issue Mar 25, 2020 · 9 comments
Labels
a: existing-apps Integration with existing apps via the add-to-app flow c: performance Relates to speed or footprint issues (see "perf:" labels) P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team

Comments

@xster
Copy link
Member

xster commented Mar 25, 2020

On a simple test, it takes ~8s to connect to an already running flutter observatory via flutter attach. Though ~3s was from cleaning up the devfs and closing the process.

Log gist at https://gist.github.com/xster/5becc4937c2cef8d5e84660d0601abd1

There's a 2397ms apparently pure Dart part in the middle of the process too.

We should optimize.

@xster xster added tool Affects the "flutter" command-line tool. See also t: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) a: existing-apps Integration with existing apps via the add-to-app flow labels Mar 25, 2020
@jonahwilliams
Copy link
Member

It's initializing the frontend server and compiling the application code. This process is bootstrapped from the already compiled app if available.

@xster
Copy link
Member Author

xster commented Mar 27, 2020

Ah, that sounds reasonable. You're saying that middle 2s+ would be drastically reduced in profile mode right?

@jonahwilliams
Copy link
Member

Yeah, under profile mode we wouldn't attempt to compile anything - just connect

@jonahwilliams
Copy link
Member

there also shouldn't be any devfs work in profile mode - so if that is case there is definitely a bug

@jmagman jmagman added this to Awaiting triage in Tools - team support review via automation Mar 27, 2020
@zanderso
Copy link
Member

@xster @jonahwilliams were either of you able to verify whether extra work was being done in profile mode?

@zanderso zanderso moved this from Awaiting triage to Engineer reviewed in Tools - team support review Apr 15, 2020
@xster
Copy link
Member Author

xster commented Apr 15, 2020

It was not. The purpose of flutter attach is to connect to a previously detached full-flutter app or to an add-to-app module in debug rather than profile mode.

@zanderso
Copy link
Member

Is there anything actionable left in this issue, or should we close it?

@xster
Copy link
Member Author

xster commented Apr 16, 2020

I think we should profile whether any of the 8s spent during the attach can be improved on. If not, we should provide clearer output on what the tool is doing while the user's waiting.

@jmagman jmagman added the P2 Important issues not at the top of the work list label Aug 18, 2020
@jonahwilliams
Copy link
Member

@xster would you be willing to try this again? I've landed a few optimizations to devFS initialization which should help. Preferably one that is uncached and one that is fully cached

@flutter-triage-bot flutter-triage-bot bot added team-tool Owned by Flutter Tool team triaged-tool Triaged by Flutter Tool team labels Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: existing-apps Integration with existing apps via the add-to-app flow c: performance Relates to speed or footprint issues (see "perf:" labels) P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team
Projects
Tools - team support review
  
Engineer reviewed
Development

No branches or pull requests

4 participants