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

Improve memory stats #5

Closed
pfalcon opened this issue Dec 29, 2013 · 6 comments
Closed

Improve memory stats #5

pfalcon opened this issue Dec 29, 2013 · 6 comments

Comments

@pfalcon
Copy link
Contributor

pfalcon commented Dec 29, 2013

With fix to #2, it's now possible to fix alloc stats collection, but instead of throwing a random patch, I'd like to be sure there's good understanding what we can/want to measure.

  1. We can measure total allocation size. Why, it's useful to know memory throughput of an app. This is what uPy appear to measure currently with total_bytes_allocated. Except that realloc case is not correct. Memory usage grows not by new_num_bytes, but by difference between new and old size. And by logic, this metric is monotonically growing, so, (new_num_bytes - old_num_bytes) < 0 should be handled separately (specifically, ignored).
  2. Besides total memory allocation, it makes sense to know current memory allocation (that allows to know how much is free in particular). It takes having another var, updated also on m_free (and on realloc w/o any conditions). So, this metric is expected to be 0 at app end and proper interpreter shutdown.
  3. Finally, it's useful to know peak memory usage we achieved. This will answer question if particular app can run on particular heap (== on particular hardware). This would be just max(current usage).

Hopefully, all these metrics are useful (well, I'd say that 2&3 are more useful than 1). If you agree, I can implement them.

@Neon22
Copy link
Contributor

Neon22 commented Dec 29, 2013

That sounds useful. Obviously 3 will vary on a specific run - so maybe if user has a unit test of some kind you could also collect data for statistical memory size ?
Would a specific test be useful for this ?

@pfalcon
Copy link
Contributor Author

pfalcon commented Dec 29, 2013

I don't think these will be useful for unit testing, it's more useful for (manual) testing of interpreter optimization and for particular apps optimization. Though maybe I just miss your usecase.

@dpgeorge
Copy link
Member

At the moment the unix version of uPy does not free memory. The GC doesn't work for it. I concentrated more on the STM version, and getting the GC working on that first. The unix version is just to test functionality of the Python bit.

But I agree it would be nice to have proper memory accounting in the unix version so we could test heap memory usage etc. We could also make the GC work on the unix version (not too hard to do). Note that a 64 bit machine is going to use around 2x more memory than the 32 bit MCU, but that can be accounted for.

The first thing to do is get the GC working, since then objects will actually be freed, and the accounting will make sense. At the moment, free is hardly ever called.

@Neon22
Copy link
Contributor

Neon22 commented Dec 30, 2013

My use case is:
User writes python code and runs it in unix/windows. Code stats report on memory usage. This information tells user that their code will use almost all available target RAM 90% of the time but will occasionally exceed the memory constraints of the target.
Therefore the user chooses to (say) sacrifice code size for speed and leave two functions running interpreted instead of unrolled into C code.

By running the code numerous times or (say) through all test cases defined in test suite, the user is able to build a statistical case for memory useage. Bearing in mind this is probably still not accurate because not all possible routes through the code may be exercised. But it will still be helpful.

@pfalcon
Copy link
Contributor Author

pfalcon commented Dec 31, 2013

@Neon22: Yeah, that's exactly the usecase I have in mind, thanks for clarification. Another usecase is to test/validate changes to the interpreter itself. For example, in #8 I propose to store hash/length together with a string, by running "before" and "after" thru memory stats we can quantitatively assess the impact (and yes, that requires testcases for various use types).

@pfalcon
Copy link
Contributor Author

pfalcon commented Jan 4, 2014

Described stats are now collected. It might be useful to expose them to Python side, but that can wait another ticket.

EcmaXp added a commit to OpenPythons/micropython-opencom-legacy that referenced this issue Aug 28, 2016
nubcore added a commit to nubcore/micropython that referenced this issue Jan 13, 2017
nickzoic pushed a commit to nickzoic/micropython that referenced this issue Sep 1, 2018
acamilo referenced this issue in acamilo/micropython Sep 24, 2018
cfobel pushed a commit to sci-bots/micropython that referenced this issue Nov 26, 2019
Fixed errors in Build instructions in README.md
tannewt referenced this issue in tannewt/circuitpython Dec 18, 2019
kamtom480 pushed a commit to kamtom480/micropython that referenced this issue Jun 1, 2020
kamtom480 pushed a commit to kamtom480/micropython that referenced this issue Nov 19, 2020
It was incorrect to NULL out the pointer to our heap allocated buffer in
`reset`, because subsequent to framebuffer_reset, but while
the heap was still active, we could call `get_bufinfo` again,
leading to a fresh allocation on the heap that is about to be destroyed.

Typical stack trace:
```
#1  0x0006c368 in sharpdisplay_framebuffer_get_bufinfo
#2  0x0006ad6e in _refresh_display
#3  0x0006b168 in framebufferio_framebufferdisplay_background
micropython#4  0x00069d22 in displayio_background
micropython#5  0x00045496 in supervisor_background_tasks
micropython#6  0x000446e8 in background_callback_run_all
micropython#7  0x00045546 in supervisor_run_background_tasks_if_tick
micropython#8  0x0005b042 in common_hal_neopixel_write
micropython#9  0x00044c4c in clear_temp_status
micropython#10 0x000497de in spi_flash_flush_keep_cache
micropython#11 0x00049a66 in supervisor_external_flash_flush
micropython#12 0x00044b22 in supervisor_flash_flush
micropython#13 0x0004490e in filesystem_flush
micropython#14 0x00043e18 in cleanup_after_vm
micropython#15 0x0004414c in run_repl
micropython#16 0x000441ce in main
```
When this happened -- which was inconsistent -- the display would keep
some heap allocation across reset which is exactly what we need to avoid.

NULLing the pointer in reconstruct follows what RGBMatrix does, and that
code is a bit more battle-tested anyway.

If I had a motivation for structuring the SharpMemory code differently,
I can no longer recall it.

Testing performed: Ran my complicated calculator program over multiple
iterations without observing signs of heap corruption.

Closes: micropython#3473
kamtom480 pushed a commit to kamtom480/micropython that referenced this issue Feb 22, 2021
dpgeorge pushed a commit that referenced this issue May 30, 2021
asan considers that memcmp(p, q, N) is permitted to access N bytes at each
of p and q, even for values of p and q that have a difference earlier.
Accessing additional values is frequently done in practice, reading 4 or
more bytes from each input at a time for efficiency, so when completing
"non_exist<TAB>" in the repl, this causes a diagnostic:

    ==16938==ERROR: AddressSanitizer: global-buffer-overflow on
    address 0x555555cd8dc8 at pc 0x7ffff726457b bp 0x7fffffffda20 sp 0x7fff
    READ of size 9 at 0x555555cd8dc8 thread T0
        #0 0x7ffff726457a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
        #1 0x555555b0e82a in mp_repl_autocomplete ../../py/repl.c:301
        #2 0x555555c89585 in readline_process_char ../../lib/mp-readline/re
        #3 0x555555c8ac6e in readline ../../lib/mp-readline/readline.c:513
        #4 0x555555b8dcbd in do_repl /home/jepler/src/micropython/ports/uni
        #5 0x555555b90859 in main_ /home/jepler/src/micropython/ports/unix/
        #6 0x555555b90a3a in main /home/jepler/src/micropython/ports/unix/m
        #7 0x7ffff619a09a in __libc_start_main ../csu/libc-start.c:308
        #8 0x55555595fd69 in _start (/home/jepler/src/micropython/ports/uni

    0x555555cd8dc8 is located 0 bytes to the right of global variable
    'import_str' defined in '../../py/repl.c:285:23' (0x555555cd8dc0) of
    size 8
      'import_str' is ascii string 'import '

Signed-off-by: Jeff Epler <jepler@gmail.com>
sblaksono pushed a commit to sblaksono/micropython that referenced this issue Jun 15, 2021
Add frame background subtraction strings
ksekimoto pushed a commit to ksekimoto/micropython that referenced this issue Jul 16, 2021
asan considers that memcmp(p, q, N) is permitted to access N bytes at each
of p and q, even for values of p and q that have a difference earlier.
Accessing additional values is frequently done in practice, reading 4 or
more bytes from each input at a time for efficiency, so when completing
"non_exist<TAB>" in the repl, this causes a diagnostic:

    ==16938==ERROR: AddressSanitizer: global-buffer-overflow on
    address 0x555555cd8dc8 at pc 0x7ffff726457b bp 0x7fffffffda20 sp 0x7fff
    READ of size 9 at 0x555555cd8dc8 thread T0
        #0 0x7ffff726457a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
        micropython#1 0x555555b0e82a in mp_repl_autocomplete ../../py/repl.c:301
        micropython#2 0x555555c89585 in readline_process_char ../../lib/mp-readline/re
        micropython#3 0x555555c8ac6e in readline ../../lib/mp-readline/readline.c:513
        micropython#4 0x555555b8dcbd in do_repl /home/jepler/src/micropython/ports/uni
        micropython#5 0x555555b90859 in main_ /home/jepler/src/micropython/ports/unix/
        micropython#6 0x555555b90a3a in main /home/jepler/src/micropython/ports/unix/m
        micropython#7 0x7ffff619a09a in __libc_start_main ../csu/libc-start.c:308
        micropython#8 0x55555595fd69 in _start (/home/jepler/src/micropython/ports/uni

    0x555555cd8dc8 is located 0 bytes to the right of global variable
    'import_str' defined in '../../py/repl.c:285:23' (0x555555cd8dc0) of
    size 8
      'import_str' is ascii string 'import '

Signed-off-by: Jeff Epler <jepler@gmail.com>
alphaFred referenced this issue in alphaFred/micropython Aug 28, 2021
* mimxrt/1170evk: Refactored clock configuration.

* mimxrt/1170evk: Fixed formatting.

* Revert "mimxrt/1170evk: Fixed formatting."

This reverts commit 021168d.

* mimxrt/1170evk: Fixed findings.

* mimxrt/1176_evk: Refactored machine_pin.

* mimxrt/1176_evk: Refactored machine_spi.

* mimxrt/1176_evk: Fixed clock for machine_timer module.

* mimxrt/1176_evk: fixed machine pin.

* mimxrt/1170_evk: Fixed uart module.

* mimxrt/1170_evk: Fixed flash module.

* mimxrt/1170_evk: Fixed tiny usb.

* mimxrt/1170_evk: Fixed pin generation - TODO create define for 1170./mimxrt_build.sh

* mimxrt/1170_evk: Added temporary qspi config.

* mimxrt/1170_evk: Fixed linker script, updated clock config for I2C to 33MHz updated makefile.

* mimxrt/1170_evk: working state of tinyusb update.

* mimxrt/1170_evk: Fixed issues after rebase.

Co-authored-by: robert-hh <robert@hammelrath.com>
j-devel pushed a commit to AnimaGUS-minerva/micropython that referenced this issue Dec 8, 2021
[riot-patch-part-1] Integrate standalone build support
peterharperuk added a commit to peterharperuk/micropython that referenced this issue Jul 21, 2022
Currently you can only set the "pm" power management value. Allow it to
be queried as well.

Requries a change to the cyw43 driver

Fixes micropython#5

Signed-off-by: Peter Harper <peter.harper@raspberrypi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants