Skip to content
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.

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

[AssetBundle/Engine] Large bundle loading cause frames missing #94464

Open
AlexV525 opened this issue Dec 1, 2021 · 14 comments
Open

[AssetBundle/Engine] Large bundle loading cause frames missing #94464

AlexV525 opened this issue Dec 1, 2021 · 14 comments
Labels
engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list perf: speed Performance issues related to (mostly rendering) speed team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@AlexV525
Copy link
Member

AlexV525 commented Dec 1, 2021

Large bundle (51M JSON file) loading still causes a couple of frames missing.

Related issues/PRs

Details

195b6eba0e84e303bf89e30db5bd30a

@AlexV525 AlexV525 added the from: performance template Issues created via a performance issue template label Dec 1, 2021
@AlexV525
Copy link
Member Author

AlexV525 commented Dec 1, 2021

cc @zhongwuzw @dnfield

@dnfield
Copy link
Contributor

dnfield commented Dec 1, 2021

What's the example?

@AlexV525
Copy link
Member Author

AlexV525 commented Dec 1, 2021

What's the example?

😢 Sorry, updated in the top comment.

@zhongwuzw
Copy link
Member

zhongwuzw commented Dec 1, 2021

Assets are loaded on the UI thread (executes all Dart code for the root isolate). So if assets size is large, I/O overhead can not be neglected. Seems we tried to move it to other threads. :)

@dnfield
Copy link
Contributor

dnfield commented Dec 1, 2021

Ahh sure this will definitely be slow.

Is this a real thing though? I agree an application can do this but it doesn't seem necessary - there are several other serious reasons to avoid this pattern.

Another question is how much time is spent on copying the data rather than loading it from disk.

@AlexV525
Copy link
Member Author

AlexV525 commented Dec 1, 2021

Is this a real thing though? I agree an application can do this but it doesn't seem necessary - there are several other serious reasons to avoid this pattern.

This is the data from some random COVID sites, loading this costs hugely, and yes not only Flutter will be stuck at present.
I could imagine some scenarios when users use Flutter to develop games, which requires assets actively.

Another question is how much time is spent on copying the data rather than loading it from disk.

Not sure if File would help, I'm not familiar with that case.

@AlexV525
Copy link
Member Author

AlexV525 commented Dec 1, 2021

Maybe we should be happy that this process only makes one frame missing? 😕

@dnfield dnfield added engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list labels Dec 1, 2021
@dnfield
Copy link
Contributor

dnfield commented Dec 1, 2021

If you include a file as an Asset on Android, I do not believe you can use the File API to open it. You could, however, use a plugin to open it.

I'm not recommending you do that, and I'm not entirely convinced that the asset IO should be synchronous, but I'm also not really swayed by this example - having a 50mb asset in your APK just seems like a really bad idea, and trying to load it all at once is going to really hurt on low end phones whether it's sync or async - forget even trying to parse the JSON into a usable data structure.

@maheshmnj maheshmnj added in triage Presently being triaged by the triage team and removed from: performance template Issues created via a performance issue template labels Dec 2, 2021
@maheshmnj
Copy link
Member

Hi @AlexV525, Thanks for filing the issue. Provided that loading large files will block the UI thread and will result in frame loss. I believe this is working as intended and it is safe to close the issue.
If anyone disagrees feel free to write in the comments and I will reopen it.

Thank you.

@maheshmnj maheshmnj added r: invalid Issue is closed as not valid and removed in triage Presently being triaged by the triage team labels Dec 2, 2021
@dnfield
Copy link
Contributor

dnfield commented Dec 2, 2021

I think we can leav ethis open for further discussion.

@dnfield dnfield reopened this Dec 2, 2021
@dnfield dnfield removed the r: invalid Issue is closed as not valid label Dec 2, 2021
@AlexV525
Copy link
Member Author

AlexV525 commented Dec 3, 2021

trying to load it all at once is going to really hurt on low end phones whether it's sync or async - forget even trying to parse the JSON into a usable data structure.

Forget about the bundle, what's the best choice when loading such huge data? As I mentioned above, in this case, the data is from some COVID site, which includes plenty of statistics info. If any COVID app wants to use it, they'll try to load and parse it. The data might not be loaded every time, then when we are involved with the cache mechanism, we'll back to the start, using a plugin maybe.

@dnfield
Copy link
Contributor

dnfield commented Dec 3, 2021

That would be loaded from the network though right?

After downloading I guess you could chunk it out somehow instead of parsing the whole thing at once.

@AlexV525
Copy link
Member Author

AlexV525 commented Dec 3, 2021

That would be loaded from the network though right?

Indeed.

After downloading I guess you could chunk it out somehow instead of parsing the whole thing at once.

I'm not sure it'll work in most cases since it's a simple JSON?

@dnfield
Copy link
Contributor

dnfield commented Dec 3, 2021

Network IO will always be sync.

You could manually find offsets of sublists. Or you could preprocess the json in some server rather than on device

@maheshmnj maheshmnj added passed first triage perf: speed Performance issues related to (mostly rendering) speed labels Dec 3, 2021
@flutter-triage-bot flutter-triage-bot bot added team-engine Owned by Engine team triaged-engine Triaged by Engine team labels Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list perf: speed Performance issues related to (mostly rendering) speed team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
None yet
Development

No branches or pull requests

5 participants