Optimize AssetRegistry class

[willeastcott]

The AssetRegistry class is incredibly inefficient. This PR massively optimizes it.

Before (~62ms to collapse a node in the 3D Tiles engine):

After (~3ms to collapse a node in the 3D Tiles engine):

Note, there is an API change here. AssetRegistry#find now returns undefined instead of null if no match is found. This matches how Array#find works.

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

[slimbuck]

such a huge improvement!

[vogloblinsky]

👏🏻

[Maksims]

Looks like it was inneficient build of names-asset index, which was using indices within list of assets instead of ID's of assets.
While findAll and find methoda used such indices and were really fast.
Now it has to iterate through all assets and does not use indices.
Also it allocates arrays from set on many of those operations.

How does this perform on projects with thousands of assets? I'm afraid it can be slower and lead to GC stalls.
More efficient way of updating cache and names index without iterator - will improve the bottleneck and will not reduce the performance with many assets in a project.

[willeastcott]

@Maksims Array.from/find/filter are extremely fast+optimized. ArrayRegistry#find/findAll won't be as fast as they were, but they're still very fast. The important thing is that AssetRegistry#remove is now no longer rebuilding huge looks up indexes every time it's called. This PR is basically a balance between simplicity and performance (I could have introduced more Maps but chose not too until it can be demonstrated there's an actual problem).

[kungfooman]

How does this perform on projects with thousands of assets? I'm afraid it can be slower and lead to GC stalls.

IMO it's a generic issue with Set's so far, they expose no filter/find/map methods. Hopefully in ES7, in the meantime we can just keep our eyes open for efficient/non-GC-causing polyfills.

Edit: Just made up a quick polyfill, without testing too much, but the principle should be clear:

Set.prototype.filter = function(test) {
    const ret = [];
    this.forEach((val, idx, set) => {
        if (test(val, idx, set)) {
            ret.push(val);
        }
    });
    return ret;
}
const nums = new Set([1, 2, 3, 4, 5, 6, 7, 8]);
nums.filter(_ => _ > 5);

Edit 2: found something official: https://github.com/tc39/proposal-collection-methods

IMO we should aim for a polyfill for this (less GC).

[yohami]

_assets is not longer browsable, any way to retrieve the list manually?

[willeastcott]

const assets = this.app.assets.list();

[Maksims]

Coming back to optimizations, unfortunately lack of indexing lead to major performance regression when using find and findAll.
In large projects with thousands of assets (tested on project with 2.5k of assets).
In engine 1.63.6 it used to take bellow 0.01ms for calling find (when hitting asset at the end of list) or findAll.
In engine 1.64.3+ it takes now 0.2ms for the same call. That is ~20x times regression in performance.

[willeastcott]

You're calling AssetRegistry#find/findAll every frame?

[kungfooman]

I guess the Set polyfill would solve that? Is it the Array.from overhead + GC?

[Maksims]

The particular project has a loader for custom level format and uses names of assets for referencing. And during loading it takes significantly longer to find all assets due to this performance regression.
Additional index by name as it was before (but without awful part of re-indexing), just like for url - will fix this problem.

[willeastcott]

Wanna do a PR? 😉

[Maksims]

Wanna do a PR? 😉

#5480 (review)