Extra timing on hot reload #104242
Extra timing on hot reload #104242
Conversation
flutter-dashboard
bot
added
the
tool
|
I think the extra timing information is fine and readable. I'm not sure how this would show up in IDEs, so I would defer to @jacob314 and @DanTup . Also FYI @christopherfujino |
|
For VS Code, any non-JSON in stdout is just printed as-is to the Debug Console, so this should just show up as desired with the new text :-) |
|
Re-ran Mac customer_testing, looks like a network flake |
|
Fyi @InMatrix, @redbrogdon. I do like giving users a bit more visibility where their reload time went. However, users may be unclear what compile, reload, and reassemble mean. One idea would be to rename the steps as |
|
@jensjoha you'll actually have to rebase upstream to fix the Mac customer_testing failure |
jensjoha
force-pushed
the
extra_timing_on_hot_reload
branch
from
405810e to
23bdee4
Compare
|
@christopherfujino thanks -- the test passes now =) @jacob314 @InMatrix @redbrogdon: Any verdict on the wording? Should it be kept as-is, or should it be changed (to the suggested, or something else?) |
|
Since most Flutter users don't know and should not need to care about what happens behind a hot reload, the value of the extra timing information is probably allowing them to file more informative bugs when they encounter exceptionally slow hot reloads. Is this the intended use case? If this information is instead mostly for engineers working on Dart or Flutter itself, I'd vote for providing that information upon request (e.g., through a |
In my mind it serves both purposes. |
|
I am planning to merge this tomorrow. Let me know if you think I should hold it off. |
|
LGTM. Thank you for your clarification. |
This PR adds extra timings for a hot reload. As an example, before a user might see > Performing hot reload... > Reloaded 1 of 788 libraries in 554ms. With this PR it would instead be something like > Performing hot reload... > Reloaded 1 of 788 libraries in 554ms (compile: 33 ms, reload: 153 ms, reassemble: 310 ms).
6 participants
This PR adds extra timings for a hot reload.
As an example, before a user might see
With this PR it would instead be something like
Rationale:
This will make it clearer where time is spent in regards to a hot reload, making it more obvious where we could start looking for improvement opportunities.
It could conceivably also allow one to make some high-level conclusions. As an example say one saw hot reloads taking 5+ seconds - if the details said 4 seconds was spent on compiling with the other numbers being more "in line" with normal, what's very different from all numbers "just" being 10 times slower than what one might expect. In fact I recently spent a day trying to chase down bad hot reload performance I saw in a screencast just to find that the computer used was under very heavy load from recording the screen and video etc. I would expect that having these numbers would have allowed me to make such a conclusion sooner.
Note, btw, that the three extra numbers were the ones identified in https://medium.com/flutter/flutter-hot-reload-f3c5994e2cee as the steps taking the most time, so I'll argue that these are the - at least for now - relevant ones.
Note: This is clearly user-facing. I don't know if there's any specific policy here. It could be moved to only be printed in verbose mode, but it would make it less useful.