perf(hogql): Fix C++ parser leaks #18022
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.
Problem
There were some memory leaks left in #17659.
Here's how the Python parser behaves:
And here's how the C++ one used to:
Changes
There were two classes of memory leaks I identified:
PyObject*
s that weren't decremented. I knew about when merging perf: Parse HogQL with C++ for a huge speedup #17659, just left that for later.delete
d. I thought the parser class destructor handled this, but it didn't.Parsing looks like this now:
How did you test this code?
Added
MemoryLeakTestMixin
that does what it says on the tin. Half of theTestParser
tests fail with that mixin onmaster
, all pass with the fixes here.