Log: delete source from builder cache when finalize #14475
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The cache of logs in Log::Builder is using a WeakRef for the Log object to not prevent the GC from collecting them, that would leak memory, but it keeps the hash entry, which also leaks, keeping strings (potentially dynamically allocated) along with entries in the hash (infinitely growing hash).
This patch adds a finalizer to Log, as suggested by @beta, to remove the log entry from the builder cache when a Log object is collected.
The runtime improvement for the sample from #14473 is dramatic. It looks almost flat with a mere 5s versus 67s for the current master at 100 000 iterations.
Note: this is a workaround to fix the leak, that can be monkey-patched in applications that need it, not a proper solution to the problem.