Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[query] Fix bad memory leak triggered by set contains IR function #10451

Merged
merged 1 commit into from May 6, 2021

Conversation

tpoterba
Copy link
Contributor

@tpoterba tpoterba commented May 5, 2021

CHANGELOG: Fixed a memory leak triggered by `hl.literal(...).contains(...)

This bug is present elsewhere in the code generator, but the set
contains function is probably the worst place for it to happen.
This leads to a full copy of the set where the binary search
is executed.

The core problem was a bug in PArrayBackedContainer not casting
its codes properly, leading to the no-op coerce logic in PCanonicalArray
being bypassed in favor of the generic copy-the-world implementation of
store. The test I have added catches the memory leak, but now fails at
compile time at the assertion to PCanonicalArray.store instead.

CHANGELOG: Fixed a memory leak triggered by `hl.literal(...).contains(...)

This bug is present elsewhere in the code generator, but the set
contains function is probably the worst place for it to happen.
This leads to a full copy of the set where the binary search
is executed.

The core problem was a bug in PArrayBackedContainer not casting
its codes properly, leading to the no-op coerce logic in PCanonicalArray
being bypassed in favor of the generic copy-the-world implementation of
`store`. The test I have added catches the memory leak, but now fails at
compile time at the assertion to `PCanonicalArray.store` instead.
@danking danking merged commit 0098488 into hail-is:main May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants