Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Oct 5, 2021

@google-cla google-cla bot added the cla: yes label Oct 5, 2021
@goderbauer
Copy link
Member

#22077 just came up in the triage, which looks related.

@dnfield
Copy link
Contributor Author

dnfield commented Oct 20, 2021

Talked about this offline a bit. We don't have any clear evidence that this will help things. This only affects a relatively narrow set of cases: image loading, SVG loading, or otherwise loading data packaged as an asset. This data is typically small, so loading it is typically fast, and the thread hop would likely if anything regress performance.

We could definitely construct an example where this makes a difference (e.g. loading some large asset file), but it's unusual for applications to include large assets.

We should add some tracing around calls to this and try to better understand how much it actually costs/when it's a problem.

@dnfield
Copy link
Contributor Author

dnfield commented Oct 21, 2021

This now has a test, but still needs a little bit of checking to see if it actually improves or regresses benchmarks.

Conceptually it seems like the right thing to do. It should probably help when larger assets, such as flare/rive animations are used.

@dnfield
Copy link
Contributor Author

dnfield commented Oct 29, 2021

Looking at startup traces, I think this would probably hurt, particularly on low end phones.

We rely on being able to read some manifest files very quickly this way - traces even on a low end phone are on the order of us/ns. If we have to do thread hops for that it will almost certainly be slower on 4 core devices.

However, it would probably still be a good idea to do this after we've started rendering frams.

@dnfield
Copy link
Contributor Author

dnfield commented Nov 3, 2021

In looking at traces, I can find lots of examples where the current behavior actually helps us and I have not really found any clear ones where it's hurting us. Going to close this for now, not really clear how to make it help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants