Skip to content

Fix memory leak in pickle creation#213

Merged
MatthieuDartiailh merged 10 commits intomainfrom
pickle-mem-leak
Jul 4, 2024
Merged

Fix memory leak in pickle creation#213
MatthieuDartiailh merged 10 commits intomainfrom
pickle-mem-leak

Conversation

@MatthieuDartiailh
Copy link
Member

@MatthieuDartiailh MatthieuDartiailh commented Jul 2, 2024

This issue is weird. The test added is not truly representative which bothers me a lot, but I have a large use case in which it does fix an issue. I will try to understand better the root issue before merging.

The reason for the changes around __slotnames__ are not clear to me but are necessary
(cherry picked from commit 32ce246618d7adb3b1560f788b299f3657726ca1)
@codecov
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.66%. Comparing base (ee88e96) to head (c52d1f5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #213      +/-   ##
==========================================
- Coverage   97.67%   97.66%   -0.02%     
==========================================
  Files          24       24              
  Lines        1075     1070       -5     
  Branches      174      174              
==========================================
- Hits         1050     1045       -5     
  Misses         12       12              
  Partials       13       13              

@MatthieuDartiailh MatthieuDartiailh merged commit c2e07c7 into main Jul 4, 2024
@MatthieuDartiailh MatthieuDartiailh deleted the pickle-mem-leak branch July 4, 2024 06:07
MatthieuDartiailh added a commit that referenced this pull request Jul 4, 2024
* fix a memory leak related to pickling Atom subclasses

The reason for the changes around __slotnames__ are not clear to me but are necessary

* cis: fix ruff invocation

* tests: update test so that it does show an improvement over main
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.

1 participant