Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Sep 26, 2020

Description

Do not upload all assets on initial devFS sync. This should increase the reliability of the initial connection, even in the face of flaky devfs behavior, in addition to a moderate perf improvement.

Updates fast-start to build assets as part of the initial bundle

Requires flutter/engine#21436
Requires flutter/engine#21586
Requires flutter/engine#21611

@flutter-dashboard flutter-dashboard bot added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Sep 26, 2020
if (content.isModified || (bundleFirstUpload && archivePath != null)) {
// Only update assets if they have been modified.
if (content.isModified) {
final Uri deviceUri = _fileSystem.path.toUri(_fileSystem.path.join(assetDirectory, archivePath));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bonus perf improvement by moving this under the check

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@jonahwilliams jonahwilliams marked this pull request as ready for review October 9, 2020 17:41
String dillOutputPath,
bool fullRestart = false,
String projectRootPath,
bool skipAssets = false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not used

@jonahwilliams jonahwilliams requested a review from dnfield October 9, 2020 17:50
// Only update assets if they have been modified, or if this is the
// first upload of the asset bundle.
if (content.isModified || (bundleFirstUpload && archivePath != null)) {
if (content.isModified && !bundleFirstUpload) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: flip the sense and unindent

if (!content.isModified || bundleFirstUpload) {
  return;
}
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@dnfield
Copy link
Contributor

dnfield commented Oct 9, 2020

LGTM as well.

@jonahwilliams
Copy link
Contributor Author

Google testing passed

@jonahwilliams jonahwilliams merged commit 9a3a0dc into flutter:master Oct 9, 2020
@jonahwilliams jonahwilliams deleted the try_different_reload branch October 9, 2020 19:25
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.

5 participants