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

utils: remove custom memoize decorator #3486

Merged
merged 1 commit into from
Jan 30, 2021

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Jan 12, 2021

This replaces the custom memoize decorator in favor of the native functools.lru_cache decorator (available since py 3.2).

Since I didn't set maxsize=None, this also changes the cache size of each decorated method, which was infinite prior to these changes and is now set to 128 with the LRU algorithm.

Not sure why I had to force-push and explicitly set maxsize=128 on py <3.8, as it's the default there as well:
https://github.com/python/cpython/blob/3.7/Lib/functools.py#L458
https://github.com/python/cpython/blob/3.8/Lib/functools.py#L487

@bastimeyer bastimeyer force-pushed the cleanup/utils/memoize branch from d8c6bbf to d559065 Compare January 12, 2021 21:35
@bastimeyer
Copy link
Member Author

Btw, could the infinite cache size of session.resolve_url be the cause of #2988 (comment) ?
This basically keeps all session instance references on the method cache, so the GC can't free it up.

@beardypig
Copy link
Member

Btw, could the infinite cache size of session.resolve_url be the cause of #2988 (comment) ?
This basically keeps all session instance references on the method cache, so the GC can't free it up.

You might be on to something there. If it is that, then setting maxsize would solve it.

@back-to back-to merged commit 6b59f7d into streamlink:master Jan 30, 2021
@bastimeyer bastimeyer deleted the cleanup/utils/memoize branch January 30, 2021 06:51
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Mar 23, 2021
@bastimeyer bastimeyer mentioned this pull request Feb 21, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants