Make cleanup of OpenGL objects explicit in GLMakie #4699
Conversation
Benchmark ResultsSHA: 667094d1cce35b3e850af13c5c78264005a20878 Warning These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking.
|
GLMakie/src/GLAbstraction/GLInfo.jl
Outdated
| const FAILED_FREE_COUNTER = Ref(0) | ||
|
|
||
| function verify_free(obj::T, name = string(T)) where T | ||
| if obj.id != 0 | ||
| FAILED_FREE_COUNTER[] = FAILED_FREE_COUNTER[] + 1 | ||
| Threads.@spawn println(stderr, "Error: $name has not been freed.") | ||
| end | ||
| end |
There was a problem hiding this comment.
This is used as the finalizer now, to "error" when something hasn't been deleted. I also added a counter to query in CI. We could also de/activate that with an environment variable but I figured incrementing and integer doesn't really matter.
There was a problem hiding this comment.
TODO:
string(T) seems to be pretty slow, should be moved so it only happens if the branch is true (dragged one of my benchmarks in #4689 from ~300µs down to 800µs)
There was a problem hiding this comment.
Didn't we want to completely disable finalizers and only activate them in debug mode?
There was a problem hiding this comment.
I think registering a finalizer is somewhat expensive, so if we only use it for debugging purposes we should just disable it via a global ( not an env var)
Description
After some help from Simon we figured out that GC /
free()was the cause of issues in #4689. Specifically because free() includes a context switch and GC can trigger it whenever via finalizers, context switches can occur randomly. To fix this I'm making all cleanup of OpenGL objects explicit here, i.e. remove the need for finalizers.The current version should segfault if a finalizer deletes OpenGL objects. I'll let CI run with this so I can see if I missed anythingPassed locally, should no longer segfaultType of change
Checklist