Change Adafruit Register libraries to use a library wide shared buffer - #63
Conversation
tannewt
left a comment
There was a problem hiding this comment.
One reorganization suggestion. I do think the slower execution is worth the saved memory.
Done. I've tested this library for three days with 3-TMP117/119s, 2-BMP5xx, 3-MAX1704x and 1 DS3231 (for the bcd libraries). I don't have anything that uses i2c_struct_array. If CircuitPython ever goes multi-threaded the shared buffer might cause an issue Edit: While working with another library I realized I should have used mem.alloc rather than mem_free to compare library growth. So numbers below are redone to be more correct. New AdafruitRegister footprint savings for libraries Here are the savings over the currently deployed production version of the library:
Footprint measurement methodology In most cases even if the prod/new values changed the savings stayed the same. I'm using prod and new library files compiled with the same # Fixed version to use gc.mem_alloc()
# BMP5xx
import gc
gc.collect()
b=gc.mem_alloc()
import adafruit_bmp5xx
gc.collect()
a=gc.mem_alloc()
b-aOld mem_free wrong numbers for posterity Interesting that the savings were the same except for TMP117.
|
|
I don't see the new changes. I suspect you forgot to push them. |
I pushed them on Friday and then briefly changed to draft while I did some memory footprint anomaly checks. Then I changed it back to ready for review. Here is the link from above of the Friday Push: 17117b0 I did a second push -- that appeared empty -- today and got a new CI set of tests. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_Register to 1.13.0 from 1.12.1: > Merge pull request adafruit/Adafruit_CircuitPython_Register#63 from grgrant/move-to-module-level-alloc Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Added the following libraries: Adafruit_CircuitPython_TMF8801
@tannewt asked me to explore the possibility of a shared module wide buffer to reduce the memory footprint of the use of many register descriptors in a driver library. I also tried allocate on first use and it didn't make any improvement. The class-wide and module-wide shared buffer had the same improvement. I think module-wide would show improvement if any library used more than one non-inheritied class (like Struct and UnaryStruct) -- ROBits inherits RWBits. I'm not using any libraries that use both Struct and UnaryStruct.
I just want to say upfront that the savings was not as much as I had hoped for in libraries like MAX1704x, BMP5xx and TMP117. I'm perfectly OK if you want to just approve PR #62 which changes i2c_struct.UnaryStruct to allocate on init like all the other libraries. None of these are a must merge.
This module wide buffer change supercedes #62 and moves all buffers to a single bytearray buffer extended to the largest needed by the various register accessors.
Here are the measured savings I found:
I2C Struct/Bit(s)
Register Struct/Bit(s)
I've been running these versions on 4-5 of my temperature/humidity/CO2 sensors using MAX1704x and TMP117 (i2c_struct/Bit(s)) and also BMP5xx (register_Bit(s)) for about a week with no issues.
Also tagging @FoamyGuy as he was the author of the register_Struct/Bits versions