PERF: Break reference cycle for all Index types #27840
Conversation
topper-123
added
Index
| @@ -441,7 +442,9 @@ def _formatter_func(self): | |||
|
|
|||
| @cache_readonly | |||
| def _engine(self): | |||
| return self._engine_type(lambda: self, len(self)) | |||
| # To avoid a reference cycle, pass a weakref of self to _engine_type. | |||
| period = weakref.ref(self) | |||
There was a problem hiding this comment.
should we be using weakref in the other invocations for consistency?
There was a problem hiding this comment.
weakrefs cause some tests to fail.
I'm not sure currently if those failures are correct, could be worth an investigation. @crepererum , did you investigate if weakrefs not working is the correct behaviour?
There was a problem hiding this comment.
No, I didn't test this.
IMHO, the weakref isn't required in the other cases since we're passing a member of self and not self itself. Using a weakref seemed to make things only unnecessary complicated, since then you have just a chain of indirections (lambda->scope->weakref->self->member instead of lambda->scope->member).
|
Thanks for providing this extended fix :) |
topper-123
force-pushed
the
breaf_index_ref_cycles
branch
from
86bf0fe to
36061a1
Compare
|
I think this should be ok to pull in after the rebase? |
|
Agreed, thanks! |
* Generalize guards for index ref cycles * add issue number
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffGeneralizes the test in #27607 to be used for all index types. This finds ref cycle issues for
PeriodIndexandCategoricalIndexwhich are solved in this PR.