search

adafruit / Adafruit_CircuitPython_Requests

Requests-like interface for web interfacing
MIT License
51 stars 36 forks source link

response.text() memory consumption is too high #183

Open psitem opened 3 months ago

psitem commented 3 months ago

Getting the following exception:

memory allocation failed, allocating 32763 bytes

I have a bit more than 3X memory free at the time of making the call to response.text().

https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/c567e1d70e469c147e6cd38373aba2a8b797b7ba/adafruit_requests.py#L327

What I've found is that each pass through the iter_content loop is consuming 2X chunk_size (so 2X the content in total) and then at the self.close() another allocation happens for the full content size, where it throws the exception. Unclear to me if that's coming from .close or from the .join in content:

https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/c567e1d70e469c147e6cd38373aba2a8b797b7ba/adafruit_requests.py#L292

I've found that increasing the chunk_size to 64 gets me working at a cost of 2.5X chunk_size per pass through the iter_content loop (but half as many passes).

https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/c567e1d70e469c147e6cd38373aba2a8b797b7ba/adafruit_requests.py#L327

First observed on Adafruit CircuitPython 8.2.10 on 2024-02-14; Raspberry Pi Pico W with rp2040

Also reproduced on Adafruit CircuitPython 9.0.2 on 2024-03-28; Raspberry Pi Pico W with rp2040

justmobilize commented 3 months ago

@psitem just checking that you are using the most recent version of Requests. self.close() used to pull down additional content (if not fully read), but doesn't anymore so there shouldn't be an additional allocation.

Also the tricky thing is even though you have 3x the available memory, it may be fragmented and so you don't have a continuous block of 32763. When I'm pulling down big files, i have strategically placed gc.collect() calls

psitem commented 3 months ago

I've been through the process of adding gc.collect() calls to my code and making sure I've no code paths that can fail to release resources.

This is all based on the adafruit_requests.py file presently in main, from 0a9bb61, with a liberal additions of print( gc.mem_free() ) to track down how so much memory was getting consumed during the access of response.text.

justmobilize commented 3 months ago

And .close() is still allocating more memory?

psitem commented 3 months ago

I'm not set up to run a debugger on this so I can't observe whether it's from the .close in iter_content or the .join in content. Thinking it through, if yield works how I think it would, it's the .join.

dsblank commented 1 month ago

I too am seeing too much memory consumed by response.text (latest version here) with large responses. In addition, response.json() has a memory leak. I was able to work around that with:

response._cached = None
response._raw = None

when completed.

More importantly, I don't think adafruit_requests should have _cached and _raw. Let the user handle the memory usage.