wrap Arc<Backtrace> in Option, saving memory when backtraces are disabled#252
wrap Arc<Backtrace> in Option, saving memory when backtraces are disabled#252Firestar99 wants to merge 3 commits intoTraverse-Research:mainfrom
Arc<Backtrace> in Option, saving memory when backtraces are disabled#252Conversation
manon-traverse
left a comment
There was a problem hiding this comment.
Looks like a nice small optimization. Thanks for your contribution! :D
|
I recall checking long ago that Regardless, this saves us some bytes in the |
a0fa1d3 to
94d82f1
Compare
MarijnS95
left a comment
There was a problem hiding this comment.
Looking at this PR I still wonder why the Backtrace is wrapped in an Arc; in e45e25a I wrote that that is to support "Cloneing" the Backtrace, but glancing at the implementation that is only because of mistakes in the ownership transfer in our fn allocate() code and copies for AllocationReport which should either not receive a Backtrace at all, or borrow it temporarily while the caller can view the backtrace.
I don't have the time right now to investigate that further, feel free to do that yourself or leave the PR as is. |
This PR wraps all
Arc<Backtrace>in Option, as to not have to allocate an Arc of a disabled Backtrace each time GPU memory is allocated with backtraces are disabled. This is probably a premature optimization, but it was so easy to implement I couldn't resist :DI only tested the vulkan implementation and visualizer on Linux, I hope CI catches issues for the other platforms.