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

Optimize setting large amounts of GridMap cells by adding a batching property #1795

Open
mrjustaguy opened this issue Nov 9, 2020 · 5 comments

Comments

@mrjustaguy
Copy link

Describe the project you are working on:
Endless 3D Noise

Describe the problem or limitation you are having in your project:
Gridmap Causes 1-2 Second delay when 100 set_cell_item calls are applied in a single frame, while the calls themselves (all 100 of them combined) take a ms

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Cell Batching, I suspect the reason for the massive slowdown is because each call is processed separated from each other, and as such batching having the ability to batch the calls into a single call would give a solid speedup in these types of scenarios.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Add a property to GridMap (Tilesets are probably affected too) that is a Bool "Batching" (or some such)
If "Batching" is set to true in script, set_cell_item just accumulates it's data (Coords, item, orientation, so the stuff passed really) in an array.
When set_cells (or some such) is called Iterate over the accumulated data, setting cells according to it in one go.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Not really, as any way of setting cells has to individually add cells using set_cell_item, which just leads back to the problem.
Only alternative is to use something like https://github.com/Zylann/godot_voxel which requires recompiling the engine, and might not be as flexible as desired (for example can't have different types of meshes, which -might- in some cases be desired)

Is there a reason why this should be core and not an add-on in the asset library?:
GridMap is part of core, and any add-on using Gridmap would have the issue of not being able to send a single call for updating cells, as it would really just be a whole bunch of set_cell_item calls that are the problem in the first place..

@Calinou
Copy link
Member

Calinou commented Nov 9, 2020

Related to #1538.

@Calinou Calinou changed the title GridMap Optimizations Optimize setting large amounts of GridMap cells by adding a batching property Nov 9, 2020
@mrjustaguy
Copy link
Author

Ok, I think part of the issue is in some sort of a bottleneck in the Physics engine (Bullet) as during testing on Godot 3.2.4 beta 5 on this project:
GridMap.zip

Results in the following results:
Script runtime for both (Creating 10x10x10 cells in cube shape, then printing the time it took to run that in debugger): 2-1 ms

Profiler Results (Bullet Physics) : Physics Time ~ 83ms - frame time roughly matching
Repeating the creation of the exact same block results in the same Spike

Profiler Results (Godot Physics) : Physics time ~ 7ms - frame time ~33ms
Repeating the creation of the exact same block results in no additional spikes, so only the actual creation caused a spike, recreating the same block had no notable performance impact.

I am unsure if this is an issue that can be solved, or is a quirk of Bullet Physics causing the slowdown and if Batching would work or not.

@Calinou
Copy link
Member

Calinou commented Jan 13, 2021

@mrjustaguy See godotengine/godot#42643. Also, it's worth testing the newly merged BVH in the 3.2 branch.

@mrjustaguy
Copy link
Author

mrjustaguy commented Jan 13, 2021

Will Do.. Tomorrow

@mrjustaguy
Copy link
Author

mrjustaguy commented Jan 14, 2021

None of the seem 2 prs to have helped in any significant way (In my tests, both got were within a few ms of the original results and GP with BVH on was same when spawning as GP without BVH on), however I did notice something strange with the BVH Version..
Turning BVH on when running Godot Physics had some unusual frame spikes (from ~16ms to ~25ms) every couple dozen frames, which I didn't see with it turned off or with bullet despite the scene being empty (so didn't spawn the cells, was just spatial, an empty gridmap and a camera)

On another note, GodotPhysics Scales Many times better, in one of my Games for reference:
Godot 3.2.2 Map generation (200x200 plane, then cutting paths in it) - 400ms for script to do it's duty
GP - 2.2 sec total (hell, for what it's doing it's as good as instant)
Bullet - 15 minutes+ (I killed it after that, cuz like that's just too long)

That is an even bigger gap compared to the 10x10x10 brick test, where Godot Physics is 12 times faster, as opposed to like 1000x+ faster in the scenario above

Concluding with that, Batching would Maybe improve the Impressive GP times and Possibly get Bullet to be a tad bit better, however GP is for 95% of the cases more then fast enough to use as is (as unless you need to spawn some 500+cells per frame like possibly in some voxel game for example (for which the above mentioned addon would be better anyway), it's fast enough to keep doing so @ 60 fps)

I think this Proposal can be Closed after adding a Doc patch in the GridMap stating how it preforms poorly on Bullet compared to GP and that any games heavily depending on GridMap performance should opt for GP or the Voxel addon instead where possible, as I no longer think that batching would have a sufficient impact to performance to deem it worth the time, especially with the facts that GP is going to be the main Physics engine in Godot 4, and that the implementation suggested would probably run into the exact same bottleneck in bullet that is currently causing trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants