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

Bundles Refactor #5675

Merged
merged 28 commits into from Mar 7, 2024
Merged

Bundles Refactor #5675

merged 28 commits into from Mar 7, 2024

Conversation

Maksims
Copy link
Contributor

@Maksims Maksims commented Sep 25, 2023

Fixes #5626

Refactor and improve behavior of Bundles:

  1. Loading asset is now consistent and behaves the same regardless if it is in a bundle or not. As it will trigger bundle loading.
  2. Loading bundles now uses fetch instead of XMLHttpRequest. This provides a source for a ReadableStream.
  3. Bundle tar is now stream parsed, so its assets trigger load as they individually are loaded before whole bundle is downloaded. This spreads parsing time and improves overall loading speed.
  4. Various text assets (e.g. text, json, template, ..) now do not create Blob, which does not pollute network tab with Blob url requests and loads such assets at sync speed when it gets available from bundle. Similar approach can be applied to other binary formats (separate PR).
  5. Loading a bundle now will trigger load for all of its assets once they are downloaded.
  6. As loading and untar'ing a bundle takes little time, this PR removed WebWorker path.
  7. When loading an asset, by default if there are suitable bundles, then loaded bundles are prioritized first, then not-loaded bundles sorted by size (smallest first). This logic can be controlled by a developer using bundlesFilter.

This makes working with bundles much easier, and does not require any changes to existing projects that use bundles. It actually provides a simplification so developers can load async bundles and its assets much easier without custom code.
As well as slightly speeds up the loading as it spreads parsing as it downloads.

New APIs:

// pc.AssetRegistry
assets.load(asset, { // optional options
    bundlesIgnore: true, // force asset loading not from a bundle
    bundlesFilter: (bundles) => { // if there are suitable bundles, then this will be called provided all suitable bundles
        return bundles[0]; // you can control which bundle to load asset from
    }
})

Benchmarks #5675 (comment)

In case of no cache, loading is 18-35% faster depending on latency.

Worse latency = more benefits from bundles.
Larger bundles (more assets in them) = more benefit from bad latency.
Preloaded vs post-loaded assets improvements might vary depending on bundles contents.
When loading from cache, there is no effect (marginal error).

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@willeastcott willeastcott added performance Relating to load times or frame rate area: assets labels Sep 25, 2023
@Maksims Maksims marked this pull request as ready for review September 25, 2023 17:42
@willeastcott
Copy link
Contributor

Is is possible to have some perf numbers in the PR description please? I think the concern before was the bundles didn't reliably give better startup times than unbundled assets. So if we can now demonstrate that, I think I'd be much happier to promote bundle usage to users.

@marklundin
Copy link
Member

marklundin commented Sep 26, 2023

Given the following example, it's not totally clear to me whether bundlesIgnore takes precedence or if it loads from the first bundle specified in the bundleFilter.

// pc.AssetRegistry
assets.load(asset, {
    bundlesIgnore: true,
    bundlesFilter: (bundles) => bundles[0];
})

This could be potentially be simplified with something along the following lines

// pc.AssetRegistry
assets.load(asset, { // optional options object
    useBundle: (bundles) => { // by default (undefined) or if true and multiple assets contain a bundle, it chooses either already loaded bundle, or if none available the smallest file size bundle. If false load asset directly
      return bundles[0] // you can provide custom method to control this logic, if falsy defaults to logic as if useBundle: false 
   }
})

Examples would be:

assets.load(asset) // Asset loads from bundle if it exists in one
assets.load(asset, { useBundle : true }) // Same as above
assets.load(asset, { useBundle : false }) // Asset does not loads from bundle and loads directly
assets.load(asset, { useBundle : bundle => bundles[0] }) // Asset loads from specific bundle if it exists

@Maksims
Copy link
Contributor Author

Maksims commented Sep 26, 2023

Below are benchmarks of a specific project that I believe is a good case for bundles.
It has some preload assets to have menu screen with some interaction (renders at that moment). And post loads more assets so the application is fully playable. Assets (/files/ directory) are hard cached (will load from local cache).
Timings start from when DOM is loaded.
Averages from 5 runs on each case.

Conclusions:

In case of no cache, loading is 18-35% faster depending on latency.

Worse latency = more benefits from bundles.
Larger bundles (more assets in them) = more benefit from bad latency.
Preloaded vs post-loaded assets improvements might vary depending on bundles contents.
When loading from cache, there is no effect (marginal error).

Test Types:

no bundles, no cache: 312 requests, 12.3Mb download
no bundles, cache: 309 requests, 4.1Kb download
bundles, no cache: 65 requests (163 blobs), 12.4Mb download
bundles, cache: 62 requests (163 blobs), 4.1Kb download

Timing Names:

First Frame - when first update is called.
Renderable - when assets are preloaded and shaders compiled (second frame usually).
Playable - when preload and post load assets are loaded and app is fully playable.

Timings (good latency, Latvia - Frankfurt):

Platform Bundles Cache First Frame Renderable Playable Advantage (higher is better)
PC (fibre) 3.46s 4.63s 5.71s -
PC (fibre) 2.32s +33% 3.60s +22% 4.63s +19%
PC (fibre) 1.83s 3.14s 3.81s -
PC (fibre) 1.83s +0% 3.10s +1% 3.96s -4%
Mobile (5G) 5.45s 7.69s 9.42s -
Mobile (5G) 4.19s +23% 6.52s +15% 7.69s +18%
Mobile (5G) 3.46s 5.82s 6.88s -
Mobile (5G) 3.88s -12% 6.21s -7% 7.32s -6%
Mobile (4G) 6.62s 8.93s 11.45s -
Mobile (4G) 4.49s +22% 7.3s +18% 9.24s +19%
Mobile (3G) 7.84s 10.11s 14.46s -
Mobile (3G) 6.42s +18% 8.7s +14% 13.88s +4%

Timings (bad latency 150ms+, Latvia - San Francisco):

Platform Bundles Cache First Frame Renderable Playable Advantage (higher is better)
PC (fibre) 7.84s 9.02s 12.06s -
PC (fibre) 4.09s +48% 5.45s +40% 7.81s +35%
PC (fibre) 1.87s 3.11s 3.96s -
PC (fibre) 1.98s -6% 3.3s -6% 3.89s +2%
Mobile (4G) 13.31s 15.6s 20.67s -
Mobile (4G) 7.89s +41% 10.17s +35% 13.39s +35%

@Maksims
Copy link
Contributor Author

Maksims commented Sep 26, 2023

marklundin
When bundlesIgnore is provided, then bundlesFilter is not called as there are no bundles to filter. Same happens if asset is not in a bundle, and bundlesFilter is provided - it will not be called, as there are no bundles to filter.
bundlesFilter is called only when there are suitable bundles (that contain asset) are provided, and custom behavior is required (I still haven't came up with the case tbh).
Within the engine, we try to avoid multi-typed arguments (boolean | function), as it makes it harder to learn and IDE tooltips are harder to read.

Here is current PRs ways:

// default to be loaded from bundle if any is suitable
assets.load(asset);

// force asset to be loaded not from a bundle
assets.load(asset, {
    bundlesIgnore: true
});

// choose a random bundle (if any suitable) to load from
assets.load(asset, {
    bundlesFilter: (bundles) => {
        const ind = Math.ceil(Math.random() * bundles.length);
        return bundles[ind];
    }
});

@willeastcott
Copy link
Contributor

willeastcott commented Sep 26, 2023

Just wanted to ask another question about bundles. Obviously, these work fine in the Editor. But are they also useful for engine-only developers? So I believe I originally recommended the TAR format because anyone can open a terminal window and tar up some files. But is that all a bundle is? A vanilla TAR file? Or is there extra metadata that would make it hard for engine-only users to create?

@Maksims
Copy link
Contributor Author

Maksims commented Sep 26, 2023

Just wanted to ask another question about bundles. Obviously, these work fine in the Editor. But are they also useful for engine-only developers? So I believe I originally recommended the TAR format because anyone can open a terminal window and tar up some files. But is that all a bundle is? A vanilla TAR file? Or is there extra metadata that would make it hard for engine-only users to create?

It is just a tar file. Tests run over a bundle that I've produced using command line tar. The only rule that asset.file.url of assets in bundle should match folder structure in tar. Also bundle asset should have a list of ids in asset.data.assets. So they are perfectly usable by engine-only users, and easy to create/manage.

@devcem
Copy link

devcem commented Jan 30, 2024

Is there any update on this? This could be potentially increase our C2P values on our web games.

@willeastcott
Copy link
Contributor

Hey, @slimbuck and @mvaligursky - I want us to set aside some time this week to properly review this PR and finally get it merged. 🙏

@mvaligursky
Copy link
Contributor

Hi @Maksims - we're keen to get to review this / finalize this. When you get a chance, could you please merge main to it and resolve the conflicts.

src/framework/asset/asset-registry.js Show resolved Hide resolved
examples/src/examples/loaders/bundle.mjs Outdated Show resolved Hide resolved
src/framework/bundle/bundle.js Outdated Show resolved Hide resolved
src/framework/handlers/css.js Outdated Show resolved Hide resolved
src/framework/handlers/loader.js Outdated Show resolved Hide resolved
@Maksims
Copy link
Contributor Author

Maksims commented Mar 4, 2024

Updated this PR:

  1. Example
  2. TextDecoder is now lazily initialized same as in the other places in the engine.
  3. Added docs where needed. openBinary should be added only on specific handlers where it is necessary, so not adding it to ResourceHandler.

Tested on existing project - works as before.

@kpal81xd kpal81xd merged commit 9d17f0d into playcanvas:main Mar 7, 2024
7 checks passed
@Maksims Maksims deleted the bundles-refactor branch March 7, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: assets performance Relating to load times or frame rate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asset Bundles - improvements and making it public
9 participants