feat: allow freeing of ustring table for debugging cases#5193
Conversation
|
Thanks for the quick turnaround! But the allocations for entries and pool in |
lgritz
force-pushed
the
lg-ustringcleanup
branch
3 times, most recently
from
fbeaff2 to
778e003
Compare
|
I updated with some tightening up of the allocation logic, including shrink_to_fit on the vectors. Also, I added a print message when it does the free (for Debug builds only, and only if you have turned on the ustring cleanup), maybe you can verify that it's really running the cleanup. If it's printing that and still complaining about leaks, is it able to tell you which allocation it is that's still never freed? |
|
The cleanup code is running. It still shows that the Edit: the print message is just for now for debugging the PR? I'd prefer if it would not print anything later, even not in Debug builds. |
|
I understand why. Stay tuned, will have an amendment coming soon. |
|
@jreichel-nvidia Amendment pushed. Any better? |
|
Yes, now it works! What about the debug output? Would it be possible to get rid of that? |
Absolutely! That was only there to help you figure out for sure if the destructor was being called. Will update. |
|
Wait, I screwed this up. By overzealously using unique_ptr, I've actually converted the code to doing the deallocations I once wished to avoid. I need to rework this again, sorry. |
This patch optionally frees the contents of the ustring table as the
process is exiting. It's triggered one of two ways:
- `OIIO::attribute("ustring:cleanup", 1);`
- Environment variable `OIIO_USTRING_CLEANUP=1`
If neither is used, the teardown won't happen, thus avoiding any
pointless work when it should be exiting the process.
The purpose of this is that when using certain debugging tools, the
ustring table (which only grows and never frees) looks like a memory
leak, even though it's by design. Someimes, it's useful to make it not
appear ot leak, and you are happy for a large table to spend time
pointlessly freeing all the data.
Fixes 3913
Assisted-by: Claude Code / Sonnet 4.6
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Coalesce repeated pool block allocation logic into allocate_pool_block(). * Change all_pools and large_allocs to vectors of std::unique_ptr so clearing the vec frees the pointers automatically. * shrink_to_fit() after clearing all_pools and large_allocs. * Initialize all TableRepMap fields by default, especially the pointers. * When in debug mode, print when freeing the ustring table resources, so we are sure it happens. Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
|
I took another pass at this, simplifying quite a bit (the whole atexit was not necessary, I could have done it in the destructors all along) and also ensuring that most of the deallocations are skipped when the special flag is not set. Sorry to make you revisit this, @jreichel-nvidia, but can you try once more to check that what I have now still fixes all the false-positive leaks you were detecting? Thanks. |
2 participants
This patch optionally frees the contents of the ustring table as the process is exiting. It's triggered one of two ways:
OIIO::attribute("ustring:cleanup", 1);OIIO_USTRING_CLEANUP=1If neither is used, the teardown won't happen, thus avoiding any pointless work when it should be exiting the process.
The purpose of this is that when using certain debugging tools, the ustring table (which only grows and never frees) looks like a memory leak, even though it's by design. Someimes, it's useful to make it not appear ot leak, and you are happy for a large table to spend time pointlessly freeing all the data.
Fixes #3913
Assisted-by: Claude Code / Sonnet 4.6