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

Fixed ref-counting of Python object temporaries in unboxing code #1454

Merged
merged 3 commits into from
May 10, 2024

Conversation

oleksandr-pavlyk
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented May 9, 2024

Ref-count of arrayobj was not decremented after use in case dpnp.ndarray object was being processed.

Change the flow of managing lifetime of Python temporaries in unboxing code.
This PR replaces use of obj in NRT structure with use of arrayobj, and shifts ref-count manipulation of obj into PyUSMNdArray_ARRAYOBJ.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

Ref-count of `arrayobj` was not decremented after use in case
`dpnp.ndarray` object was being processed.
@ZzEeKkAa
Copy link
Contributor

ZzEeKkAa commented May 9, 2024

Check locally with unitrace - memory leak that @adarshyoga was reporting is resolved with this PR

@diptorupd
Copy link
Contributor

@oleksandr-pavlyk there are some compile time warnings after these changes.

@adarshyoga
Copy link
Contributor

The memory leak that was occurring with pairwise distance workload is resolved with this fix. Thanks, @oleksandr-pavlyk .

@oleksandr-pavlyk
Copy link
Contributor Author

@oleksandr-pavlyk there are some compile time warnings after these changes.

Yes, I pushed a fix for them, by casting PyUSMArrayObject * pointer to PyObject * pointer, which is always safe to do.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 86.494% (+0.01%) from 86.48%
when pulling 5dcf8af on correct-refcount-handling-fixing-memory-leak
into e36c979 on main.

@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review May 10, 2024 02:17
@diptorupd diptorupd merged commit 574daab into main May 10, 2024
71 of 75 checks passed
@diptorupd diptorupd deleted the correct-refcount-handling-fixing-memory-leak branch May 10, 2024 12:46
github-actions bot added a commit that referenced this pull request May 10, 2024
…ixing-memory-leak

Fixed ref-counting of Python object temporaries in unboxing code 574daab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants