Use weakref for bound stream _response.#3733
Conversation
This avoids creating reference cycles that can result in significant extra memory usage. The cyclic GC should clean up the cycles but avoiding them is better.
|
Thank you both for investigating this! |
|
I use this one to test on windows. It uses |
|
I see that before it grows from 51 to 84 MB, and afterward it only grows from 51 to 61 MB and then stabilizes (or grows extremely slowly). |
|
Is the first 50–60 MB spike unrelated, given that it still happens with this PR? |
|
I believe it is normal now, but I would like to hear @nascheme opinion. |
|
The exact pattern of how process memory changes is going to be platform dependent. E.g. it depends on how eager the malloc is to release memory back to the OS when it is freed. It also depends on how much memory the The important part is avoiding the reference cycle. I could add a unit test to confirm that if you wish but I think it's pretty apparent that the # ssl_context_size.py
import ssl
def get_rss():
import psutil
p = psutil.Process()
mem_info = p.memory_info()
vms = mem_info.vms
rss = mem_info.rss
return rss / 1024.0
def main():
rss = get_rss()
ctxs = []
N = 1_000
for i in range(N):
ctxs.append(ssl.create_default_context())
rss2 = get_rss()
size = (rss2 - rss) / N
print(f'size {size} kB')
if __name__ == '__main__':
main() |
|
LGTM! Thanks @nascheme and @sergey-miryanov. Would you be able to also update the changelog? |
|
Hello, if you find useful, here are some tests for this: |
Instead of holding a reference back to the Response in BoundSyncStream/ BoundAsyncStream (which creates a reference cycle), store the elapsed timedelta on the stream itself after close. Response.elapsed reads it back from self.stream via duck typing, falling back to a directly-set _elapsed value for cases like mocking. This avoids creating reference cycles that can result in significant extra memory usage. It's an alternative approach to encode/httpx#3733 and I prefer my approach, because it does not use references at all (no weakref). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
This avoids creating reference cycles that can result in significant extra memory usage. The cyclic GC should clean up the cycles but avoiding them is better.
The Python 3.14.2 release makes the cyclic GC less aggressive about freeing reference cycles and so the cycles created from the
_responseattribute can result is significant memory use (80 MB vs 420 MB). With this change, my example script goes to using 36 MB. See this CPython bug.Checklist
This change is pretty small so I didn't add a test case for it. The
_responseattribute doesn't appear to be used from outside theBound*classes. Should be no documentation update needed.