Conversation
There was a problem hiding this comment.
In theory, the expensive part should be
await writeBundle(fs.directory(fs.path.join('build', 'unit_test_assets')),
assetBundle.entries);
I check the code assetBundle.build() is just bunch of string operation, it only loads the pubspec files which we cannot avoid anyway.
There was a problem hiding this comment.
Lets double check the performance of a single test on gallery, since that has a reasonable number of assets.
The benchmark runs on the hello_world test, so we should gather numbers for that too.
There was a problem hiding this comment.
would you suggest just time several run with and without the patch?
There was a problem hiding this comment.
sounds like a good approach
There was a problem hiding this comment.
I am seeing a little bit improvement
gallery smoke test 113.4 -> 107
hello world 2.98 -> 2.86
edit I ran ~8 times each case
There was a problem hiding this comment.
What are the numbers before the original patch?
There was a problem hiding this comment.
not sure if this is intended, the isModifiedAfter will return true if we have not access access the file stat before. no matter what lastModified is.
There was a problem hiding this comment.
This intended for the other usage of devfs, but is unfortunate in this case. I would add a comment documenting this behavior here
There was a problem hiding this comment.
use Iterable.whereType< DevFSFileContent >()
There was a problem hiding this comment.
I would early return to reduce nesting.
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
LGTM, lets see how the benchmarks do on the devicelab after this is submitted
4 participants
Description
This pr changed the asset build behavior, it will first gather all the assets, check if any assets has new modify time. if there is, it will rebuild it. otherwise, skip.
Related Issues
#31511
Tests
TBD
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?