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

RD::texture_create_shared_from_slice become very slow when used extensively on a Texture2DArrayRD #98733

Open
ze2j opened this issue Nov 1, 2024 · 1 comment

Comments

@ze2j
Copy link
Contributor

ze2j commented Nov 1, 2024

Tested versions

  • Reproducible in Godot v4.3.1.rc (725f50752)

System information

Ubuntu 22.04.5 LTS 22.04 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1070 (nvidia; 535.183.01) - Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz (8 Threads)

Issue description

I noticed a performance issue in RenderingDevice::texture_create_shared_from_slice. In my game I use a Texture2DArrayRD and create 9 shared textures for each layer (the mipmaps). When I end up with 2000 shared textures, my game starts jittering a lot.

I profiled my game using Tracy and the issue is with this line of code: Texture texture = *src_texture; in RenderingDevice::texture_create_shared_from_slice. 80% of the duration of RenderingDevice::texture_create_shared_from_slice can be spent there.

The Texture class has a member named slice_trackers which is a hashmap tracking the shared textures (correct me if I am wrong). It's the copy of this hashmap which slows down the Texture copy. On my system the copy takes about 50ns when the hashmap is almost empty and can reach 200µs or more when it contains 2500+ elements:

slice_trackers copy
As I can create 100 shared textures per frame, I get frames at 40ms or more...

IMO this can be solved by not copying the slice_trackers member. slice_trackers is only used by the owner texture and not by the shared ones. In the current implementation we have Texture texture = *src_texture; and then texture->slice_trackers.clear(); comes later in RD::_texture_make_mutable.

I am considering creating a PR replacing the line Texture texture = *src_texture; by something like Texture texture = src_texture->duplicate_as_shared_texture(); where Texture duplicate_as_shared_texture() const copies every members except slice_trackers.

I tested this fix and I got the expected results (this the durations of RenderingDevice::texture_create_shared_from_slice):
slice_trackers no copy

I am worried about maintainability and if you would like to support such a use case in the first place. So before creating a MRP (which require some work) and a PR I would like to get some feedback first.

Steps to reproduce

  • Create a Texture2DArrayRD with 1024 layers
  • Call 10 times RD::texture_create_shared_from_slice for each layer

Minimal reproduction project (MRP)

Will do if my use case is not too project specific.

@clayjohn
Copy link
Member

clayjohn commented Nov 1, 2024

Performance improvements are almost always welcome. I would have to see a PR to properly judge if the additional complexity/maintenance burden is justifiable though.

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

2 participants