Skip to content

Conversation

jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Oct 10, 2020

Description

On every hot reload, the flutter tool must file stat each asset in the bundle. With a large number of assets or a slow file system, this can take 20 - 30 ms. Do this operation while the flutter tool is waiting for a response from the frontend_server.

No tests updated since this is only a timing update. Any difference in behavior will be shown on benchmarks

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 10, 2020
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@google-cla google-cla bot added the cla: yes label Oct 10, 2020
if (fullRestart) {
generator.reset();
}
// On a full restart, or on an initial compile for the attach based workflow,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is unchanged, but the asset stat was placed in between the pendingCompilerOutput resolution.

@jonahwilliams jonahwilliams changed the title [flutter_tools] micro-optimize devfs upload [flutter_tools] check asset files while the tool waits for the frontend_server compile Oct 10, 2020
// The tool writes the assets into the AssetBundle working dir so that they
// are in the same location in DevFS and the iOS simulator.
final String assetBuildDirPrefix = _asUriPath(getAssetBuildDirectory());
final String assetDirectory = getAssetBuildDirectory();
bundle.entries.forEach((String archivePath, DevFSContent content) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does the stat happen? Is it done as part of the isModified and size getters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// this will produce a full dill. Subsequent invocations will produce incremental
// dill files that depend on the invalidated files.
_logger.printTrace('Compiling dart to kernel with ${invalidatedFiles.length} updated files');
final Future<CompilerOutput> pendingCompilerOutput = generator.recompile(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment here explaining why the Future is awaited later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jonahwilliams jonahwilliams merged commit 408cd71 into flutter:master Oct 12, 2020
@jonahwilliams jonahwilliams deleted the bundle_timing branch October 12, 2020 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants