peterhinch / micropython-touch

A GUI for touch panel displays.
MIT License
21 stars 5 forks source link

Out of memory errors on use of Screen.REPLACE for rotating screens #13

Open bianc104 opened 1 month ago

bianc104 commented 1 month ago

I'm attempting to cycle between screen instances using a similar methodology to screen_replace.py, but I'm finding that after enough cycles, my program crashes yielding MemoryError: memory allocation failed, allocating 376 bytes (it is consistently 376 bytes)

The screens themselves are somewhat complex, consisting of about 44 widgets, however I am using the Screen.REPLACE method, which if I understand the documentation correctly is supposed to essentially mark the prior screens for clearing by the garbage collector. I do pass a callback and two small strings through to each subsequent screen instance, however the demo in screen_replace.py appears to pass some data forward similarly as well.

In my efforts to debug this, I've made use of the micropython mem_info readout prior to and after executing the screen change to ascertain memory usages. As expected, changing the screen takes considerable memory, however it appears that the majority of that memory gets re-freed prior to next button press. Some memory is not freed, however; it seems like at least 13000 bytes get taken out of free memory with each press. Presses were done with several second intervals to try to differentiate from retained memory vs memory the garbage collector had not freed yet.

After observing this, I also introduced a manual gc.collect() call on the initialization of the screen just to be sure it isn't an issue with the garbage collector not freeing the previous screen in time, but that appears to have little if any impact.

I've also tried restarting execution (stopping the program with the close button and then starting it again from the computer) and found that the memory still wasn't freed.

I would adopt some of the suggested methods of reducing library memory usage, but it seems more from the diagnostics that this is some kind of memory leak, which would render those options ineffective anyways.

Any have any suggestions, clarifications, recommendations, or further information that might allow me to resolve this issue would be greatly appreciated!

bianc104 commented 1 month ago

After absolutely extensive testing, I've isolated the issue to the button litcolor functionality. When litcolor is enabled on the buttons on the screen, it appears some or all of said screen remains in memory after switching (given the number of switches to freeze the program increased as I removed buttons, I'm thinking it was the entire screen). Removing litcolor allows me to switch the screen as many times as my finger/wrist permits, which is sufficient.

peterhinch commented 1 month ago

That is an interesting observation; I'm keen to fix this as litcolor is particularly useful on touchscreens.

Does this occur with any litcolor button, or are we considering the case where such a button causes a screen change?

peterhinch commented 1 month ago

I've pushed an update to widgets/buttons.py.

If a button had litcolor set, and that button caused a screen replacement, I think the memory used by the prior screen was not being reclaimed. This update should fix this.

Please report any observations.

bianc104 commented 1 month ago

Thank you for the quick response and patch! I've done some further experimentation today and come to the conclusion that the latest patch has mostly fixed the issue. When just pressing a button which would switch screens, it seems to reclaim memory sufficiently, however if another button with litcolor is pressed quickly enough before (or like what can happen when sliding a stylist between two closely placed buttons) it seems like I can still replicate the memory loss issue.

That being said, this is still a radical improvement as I believe the latter situation is a lot more rare and I can't readily think of an instance where that would be the desired way of inputting to the device - rather it seems more a side effect of sloppy input.

Thanks again for your help!

peterhinch commented 1 month ago

Thank you for the feedback. I'll give this some more thought. [EDIT] Have you retained the default onrelease=True in the Button constructor?

peterhinch commented 1 month ago

After further thought, I haven't figure out a way this can happen with synchronous code.

However it can happen with asynchronous code. Assume two buttons, both operating on release. One changes screen, the other launches a task which lights an LED widget on the screen after a delay. The user swipes across the two buttons.

On one scan of the screen's widgets, the two release callbacks run. The order in which they run is indeterminate, so the task may be instantiated before the screen changes. The screen change is synchronous, but by then the task is running. The task retains a reference to the LED, thereby keeping the Screen in RAM.

The quick fix is to register the task with Screen.register_task(coro(), True to ensure that the task is cancelled when the screen changes. If the task must ru to completion, then it must not hang on to widgets...

Please let me know if your callbacks run such code.