-
-
Notifications
You must be signed in to change notification settings - Fork 183
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
Resources release functions and leaks fixes to the gl_backend #428
Conversation
…ers, buffers, pipelines, render passes in GL based renderer Moved resources management from Vec to HashMap Added glDetach
…eline to the metal backend as place holder)
@not-fl3 can you review please? |
Hi, sorry for delay! Great PR, big issue that required some fix for a loooong time! I am totally into a part of the PR about deleting GPU resources. However, I am a bit conflicted about HashMaps. In most of my applications, I do access those resources very often, but I almost never reallocate things. And even if I do - those arrays do not have much data, the real data is on the GPU anyway. I do not have any profiling data to make a decision here :( |
Fedor, according to a quick benchmark, for a Billion access:
A single access using vanilla hashmap is 18ns vs 4ns, or 6ns vs 4ns for a NoHash hashmaps. a draw call shadows that by many orders of magnitude :) |
@not-fl3 what's going to be? I have high doubts that any scene will have more than 32k objects held or accessible at any frame and there are more than 100k read access. If it's more, something is going really bad there (major design flaw, bug, ...). While we can switch to HashMap<usize, Resource, NoHash>, I d still vote for vanilla HashMap. The performance "penalty" doesn't justify the added complexity and for >> 95% of cases, no one needs more than that. |
@eloraiby a sort of tangential question, but rather than using |
Sorry for delay! I got paranoid that real life delays would differ from your benchmark. But nope, HashMaps are very fast indeed, I failed to build any test case with any noticable difference between Vec and HashMap. Thanks for PR! |
Currently, pipelines and programs are not deleted and stay in memory. This pull request release the gl shader used for linking the program and add a function to release the resource.
Added a resource manager based on HashMaps to ease management (addition/removal) as well.
Note: Apple Metal backend seems to have far more leaks, but these should be addressed in a PR on their own as they require far more substantial work.