Fix issue with O(N^2) performance issue for addPart#17928
Fix issue with O(N^2) performance issue for addPart#17928spsDrop wants to merge 3 commits intoBabylonJS:masterfrom
Conversation
eliemichel
left a comment
eliemichel
left a comment
There was a problem hiding this comment.
I think that it would be easier and less invasive to change from addPart() to addParts(), then make sure to allocate mergedSplatsData only once in there with all the parts in it. You then only have a single call to this.updateData and thus only one call to _makeSplat.
The main drawback of my proposal is if you do not know ahead of time the full list of parts that you want to load and end up addParts() with a single part each time then you gain nothing and your approach is probably better.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
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/17928/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/17928/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/17928/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
4 participants
Everytime we add a part we re-call _makeSplat for every part and every splat in those parts. This results in a worsening and worsening load time instead of an expected linear growth. Here is a performance flame graph showing the growing number of calls after adding 4 splats together.