Skip to content

Reland "Speed up first asset load by encoding asset manifest in binary rather than JSON (#116662)" #116684

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.

Sign up for GitHub

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

Jump to bottom
Closed
andrewkolos wants to merge 111 commits into from
Closed

Reland "Speed up first asset load by encoding asset manifest in binary rather than JSON (#116662)" #116684

andrewkolos wants to merge 111 commits into from

Conversation

@andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Dec 7, 2022

Relands #113637, which was reverted by #116662.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@andrewkolos
initial attempt to use binary encoding for assetmanifest
9a33150
@andrewkolos
working
d8e32a6
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
9dccdc6 
…ng-for-asset-manifest
@andrewkolos
trim trailing whitespace
9c1782b
@andrewkolos
update asset decoding test
54ff2ed
@andrewkolos
Delete generated_plugins.cmake
866b972
@andrewkolos
try to use compute
3739c09
@andrewkolos
experiment with compute
7cd4ca0
@andrewkolos
remove json version of asset manifest
0d4110f
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
7c835fd 
…ng-for-asset-manifest
@andrewkolos
remove padding from asset manifest
4debadb
@andrewkolos
fix buffer length issue
ba3b36d
@andrewkolos
fix messed up method name
92dbb3d
@andrewkolos
trim trailing whitespace
e7d4d78
@andrewkolos
remove trailing whitespace
f434777
@andrewkolos
run flutter update-packages --force-upgrade
23decc2
@andrewkolos
add money_asset_manifest.bin to _legacyBinaries
13986f8
@andrewkolos
Revert "run flutter update-packages --force-upgrade"
3ffdf4d 
This reverts commit 23decc2.
@andrewkolos
restore JSON version of asset manifest
41233c2
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
a7b7c52 
…ng-for-asset-manifest
@andrewkolos
restore iteration count
04aea55
@andrewkolos
remove pointless bundle.clear call
acec0c3
@andrewkolos
normalize API
9168628
@andrewkolos
rewrite loadStructuredDataBinary to avoid event loop if data is in cache
6c2c854
@andrewkolos
provide base implementation for loadStructuredDataBinary
3c282d9 
This is to avoid breaking folks that currently extend AssetBundle.
@andrewkolos
remove asynchrony from decodeAssetManifest and weaken types
3065049
@andrewkolos
make import for ServicesBinding more specific
02d7f69
@andrewkolos
fix test
3410cc0
@andrewkolos
fix Failed image loads in debug mode test
91726f1
@andrewkolos
adjust verifyNoBinaries
35cbbb5
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
89fca21 
…ng-for-asset-manifest
@andrewkolos
add comments
dad9185
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
80941a4 
…ng-for-asset-manifest
@andrewkolos
Merge branch 'master' into add-binary-encoding-for-asset-manifest
ab5de3b
@andrewkolos
upgrade dependencies
a4d80d8
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
a3b0304 
…ng-for-asset-manifest
@andrewkolos
hmm
ef4a512
@andrewkolos
restore collection as prod dependency
19fdf34
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
23303d0 
…ng-for-asset-manifest
@andrewkolos
trying something out
05110fc
@andrewkolos
lint
c20c436
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
ce1699a 
…ng-for-asset-manifest
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
6468e99 
…ng-for-asset-manifest
@andrewkolos
fix json string parsing
ded125f
@andrewkolos
add test for AssetManifest.json
c743c78
@andrewkolos
remove dependency on collection
44993cb
@andrewkolos
add a comment to hacky try-catch
d676d75
@andrewkolos
restore collection as dev dependency
53a726e
@andrewkolos
nits
263aaec
@andrewkolos
Merge remote-tracking branch 'upstream/master' into add-binary-encodi…
61a655b 
…ng-for-asset-manifest
@andrewkolos
Speed up first asset load by encoding asset manifest in binary rather…
ca66c96 
… than JSON (flutter#113637)
@flutter-dashboard flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Dec 7, 2022
@andrewkolos andrewkolos changed the title Reland Speed up first asset load by encoding asset manifest in binary rather than JSON Reland "Speed up first asset load by encoding asset manifest in binary rather than JSON (#116662)" Dec 7, 2022
@ueman ueman mentioned this pull request Dec 11, 2022
Closed
11 tasks
@andrewkolos
Copy link
Contributor Author

andrewkolos commented Dec 16, 2022

Due to difficult to debug issues in g3, I'll be taking a different strategy to landing this change.

Instead of one change that 1) generates the new asset manifest and 2) also uses it for image resolution (with code to fallback to AssetManifest.json to prevent g3 breakages), two separate changes will be made. This way, in between these two changes, g3 tooling can be changed to produce the new asset manifest. The result is that we don't need the complicated AssetManifest.json fallback logic that is difficult to get working in g3 tests.

Development is being tracked with #117228

@andrewkolos andrewkolos mentioned this pull request Jan 19, 2023
Merged
8 tasks
@andrewkolos andrewkolos deleted the add-binary-encoding-for-asset-manifest branch April 21, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@andrewkolos