-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
improve performance of glTFLoader._applyExtensions #15913
base: master
Are you sure you want to change the base?
Conversation
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/15913/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/15913/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/15913/merge#BCU1XR#0 |
Visualization tests for WebGPU (Experimental) |
WebGL2 visualization test reporter: |
Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
Thanks for the PR. Just curious, have you seen measurable performance issues with this code? @alexchuber also recently identified an issue with the loader extension ordering that needs to be addressed. It might be good to think about both of these issues together before making a complete fix for both as they will likely touch the same code. |
Cache extensions by
_getActiveExtensionsHasOwnFunction
for a specific function name, cache extensions which are enabled and have the specific function, avoid repeated traversals
ILoaderProperty._activeLoaderExtensionFunctions
use
Map
to replace{}
for ILoaderProperty._activeLoaderExtensionFunctions, to imporve has/set/delete operationfor specific p and fName, Just ensure avoid recursive call of
applyExtension(p, fName, ...)
Let's say there is an enabled extension which name is
extensionA
and has functionloadSomeAsyc
.For a specific property
p
,First,
applyExtension(p, fName, ...)
should not be called recursively.In other words, the following trace should not occur:
Second,
extensionA.loadSomeAsyc(p, ...)
should not be called recursively.In other words, the following trace should not occur:
so, just ensue no recursive call of
applyExtension(p, 'loadSomeAsyc')
, two goals can be achieved at the same time.